Add option to configure default screen #968
Conversation
Reviewer's GuideAdds a new DefaultScreen option to the configuration, extends CConfig to parse it, implements a LoadDefaultScreen method in the UI to navigate directly to the Performance Load screen when selected, and integrates this behavior into the application startup sequence. Sequence diagram for application startup with DefaultScreen optionsequenceDiagram
participant App as CMiniDexed
participant UI as CUserInterface
participant Config as CConfig
participant Menu as CUIMenu
App->>UI: Initialize()
App->>UI: LoadDefaultScreen()
UI->>Config: GetDefaultScreen()
alt DefaultScreen == 1
UI->>Menu: EventHandler(MenuEventStepDown)
UI->>Menu: EventHandler(MenuEventSelect)
UI->>Menu: EventHandler(MenuEventSelect)
end
Class diagram for updated configuration and UI classesclassDiagram
class CConfig {
+unsigned GetMasterVolume() const
+unsigned GetDefaultScreen() const
unsigned m_nMasterVolume
unsigned m_nDefaultScreen
}
class CUserInterface {
+bool Initialize()
+void LoadDefaultScreen()
+void Process()
+void ParameterChanged()
CUIMenu m_Menu
CConfig* m_pConfig
}
CUserInterface --> CConfig : uses
CUserInterface --> CUIMenu : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @soyersoyer - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/config.h:377` </location>
<code_context>
unsigned m_nMasterVolume; // Master volume 0-127
+
+ unsigned m_nDefaultScreen; // 0 Default, 1 Performance
// Network
</code_context>
<issue_to_address>
Default value for m_nDefaultScreen should be explicitly initialized.
Initialize m_nDefaultScreen to 0 in the constructor to prevent undefined behavior from uninitialized values.
</issue_to_address>
### Comment 2
<location> `src/config.cpp:224` </location>
<code_context>
m_nMasterVolume = m_Properties.GetNumber ("MasterVolume", 64);
+
+ m_nDefaultScreen = m_Properties.GetNumber ("DefaultScreen", 0);
}
</code_context>
<issue_to_address>
Consider validating the DefaultScreen value loaded from configuration.
Without validation, m_nDefaultScreen could be set to an invalid value if the config is incorrect. Adding validation or clamping will help prevent this.
Suggested implementation:
```cpp
m_nDefaultScreen = m_Properties.GetNumber ("DefaultScreen", 0);
// Clamp m_nDefaultScreen to valid range [0, MAX_SCREENS-1]
if (m_nDefaultScreen < 0)
m_nDefaultScreen = 0;
else if (m_nDefaultScreen >= MAX_SCREENS)
m_nDefaultScreen = MAX_SCREENS - 1;
}
```
- Make sure that `MAX_SCREENS` is defined and represents the correct upper bound for valid screen indices. If the valid range is different, adjust the clamping logic accordingly.
- If `MAX_SCREENS` is not available, replace it with the correct constant or value for your application.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Build for testing: |
|
Wow, that's a great new feature. You're my personal hero for today. Thank you. After the IP address is displayed, the display jumps to TG1 – but if you then turn the encoder, you're back in the Performance - Load menu. |
|
Build for testing: |
|
You solved that very well, thank you Peter |
|
Perhaps it would be helpful to explain what "default" means in the minidexed.ini file? |
Wouldn't it be better if it automatically disappeared after, say, 3 seconds?
Sure! |
Add DefaultScreen option to the minidexed.ini 0=Default 1=Performance Load Open the Performance>Load menu if Performance Load is set
soyersoyer
force-pushed
the
defaultscreen
branch
from
d0372b6 to
061a8cd
Compare
Do you mean the network ready indicator: Yes, that would be even better. 3 seconds should be enough. |
|
Build for testing: |
soyersoyer
force-pushed
the
defaultscreen
branch
from
061a8cd to
9677019
Compare
|
Build for testing: |
|
The network ready indicator works very well. It stays on for 3 seconds, but disappears immediately when you turn the encoder. Great. |
TG1 is misleading if the menu is not on it. Show only for for 3 seconds.
soyersoyer
force-pushed
the
defaultscreen
branch
from
9677019 to
c507cb0
Compare
|
I swapped 'Network ready' with the IP because the " IP>" doesn't always fit at the 16 column LCDs. |
|
Build for testing: |
Also works on my display as described, top |
|
@probonopd , you haven't commented on this feature yet, or is there a reason why you're not integrating it? Peter |
Add DefaultScreen option to the minidexed.ini
0=Default
1=Performance Load
Open the Performance>Load menu if Performance Load is set
I think this is useful, I almost always enter here.
Summary by Sourcery
Introduce a new DefaultScreen configuration option and implement logic to open the Performance Load screen automatically on startup when selected
New Features:
Enhancements:
Documentation: