search

victronenergy / gui-v2

Other
27 stars 9 forks source link

Update settings when venus-platform provides various features #375

Closed blammit closed 4 months ago

blammit commented 1 year ago

The gui-v1 settings uses C++ helpers such as "vePlatform" to set/get various system features. In gui-v2 these are not available, and the preferred method is to do this via venus-platform.

These are the features (also marked with TODO in the code) which still need to be implemented, when they become available in venus-platform:

jhofstee commented 1 year ago

I added some of these to venus-platform. Note the CAN-bussed stats and time are intended to be polled, perhaps something is needed to make that work over MQTT as well. In dbus-spy you can now press g to fetch them.

venus-platform v1.18

mpvader commented 9 months ago

Hey @blammit do you agree to remove Debug: vePlatform.getMemInfo() and QuickView.imageCacheSize() from the list? We don't need that.

Thats in general btw: all those debug pages and debug items: please ignore them. If and when someone needs them at some time or date they'll come back.

blammit commented 9 months ago

Great, sounds good to not do those if they're not needed. Will update the list.

ReinvdZee commented 9 months ago

Hi @blammit, I'm currently working on this issue, and I started with the display controls. As I am not all that familiar with gui-v2, I could use some feedback.

Would it be sufficient for the gui-v2 to have the following dbus settings to control the display/backlight:

  1. /Settings/Gui/Brightness : The current brightness setting, in 0 to 100% (let venus-platform convert this setting in 0-100% to the supported brightness range, prevents an unnecessary dbus setting for the max brightness)
  2. /Settings/Gui/AutoBrightness : 0: disable auto brightness, 1: enable autobrightness
  3. /Settings/Gui/AutoColorMode: For the gui to use. Is venus-platform responsible for creating this setting? Other actions venus-platform should perform on this setting?

Thanks!

kwindrem commented 9 months ago

Brightness max value is actually platform dependent. For a while, I had to reset the range after GUI v1 booted to make the Raspberry PI display work properly.

I think AutoColorMode is light/dark/auto. Auto will require something in the system to actually set light or dark based on time or other factors. Maybe the light sensor in the GX Touch.

These parameters probably should be separate for each display in the system: e.g., light mode with high brightness in brightly lit area, dark mode, dimmed down a bit for say a bedroom. ....

blammit commented 9 months ago

Hi @ReinvdZee , thanks for your assistance. Regarding your suggestions:

/Settings/Gui/Brightness : The current brightness setting, in 0 to 100% (let venus-platform convert this setting in 0-100% to the supported brightness range, prevents an unnecessary dbus setting for the max brightness)

Great, yes that would be easier to just have a range of 0-100, so that gui-v2 does not need to fetch the min/max brightness values for the platform.

/Settings/Gui/AutoBrightness : 0: disable auto brightness, 1: enable autobrightness

Yes, sounds good.

/Settings/Gui/AutoColorMode: For the gui to use. Is venus-platform responsible for creating this setting? Other actions venus-platform should perform on this setting?

I think the settings are created by some startup script on the cerbo, @jepefe would know more about that.

~Ideally, when this AutoColorMode changes, venus-platform would somehow determine whether the Dark or Light setting should be used, and change /Settings/Gui/ColorScheme to 0 (Dark) or 1 (Light) as necessary.~

Ideally, when /Settings/Gui/ColorScheme changes, venus-platform would somehow determine whether the Dark or Light setting should be used, and change /Settings/Gui/AutoColorMode to 0 (Dark) or 1 (Light) as necessary.~

I don't know how this should be determined, but if there's no way to determine it then it would be better to drop that "Auto" option from the UI altogether.

blammit commented 9 months ago

@kwindrem it would be a nice bonus to have different per-display parameters, would be a good consideration for the future.

ReinvdZee commented 9 months ago

Regarding the relay features: gui-v1 controls relay 0 only when it is configured as an alarm. gui-v1 does not control relay 1 at all. As far as I can tell, the dbus-systemcalc-py service is responsible for relay 1 (always) and for relay 0 (when it is not configured as an alarm).

I can add the functionality of controlling relay 0 only in alarm mode to venus-platform, but wouldn't it make more sense to control both relays completely from venus-platform and remove relay control from dbus-systemcalc-py? There may be a good reason to have dbus-systemcalc-py control both relays as it does now, but I don't see it.

Any comments? @izak, @mpvader, @blammit

jepefe commented 9 months ago

The control of the alarm relay and the buzzer, along with alarm notifications, will be transferred to the Venus platform. Consequently, GUI-V2 will no longer need to handle these functions. Jeroen is currently reviewing the branch and will merge it upon completion.

ReinvdZee commented 9 months ago

@jepefe, thanks for the clarification! I didn't notice the branch on venus platform you were working on.

jepefe commented 9 months ago

The /Settings/Gui/ColorScheme setting is created by gui-v2: https://github.com/victronenergy/gui-v2/blob/fefdccb3a702e668ae9cf78461f86b337b51d01e/src/veqitemmockproducer.cpp#L32

and

https://github.com/victronenergy/gui-v2/blob/fefdccb3a702e668ae9cf78461f86b337b51d01e/src/veqitemmockproducer.cpp#L59

The AutoColorMode I can't find it on my Cerbo but also I can't find any code creating it. Do we beed it? Why not reuse /Settings/Gui/ColorScheme, 0=Dark, 1=Light, 2=Auto?

ReinvdZee commented 9 months ago

Because you would need /Settings/Gui/AutoColorMode == 1 + some method to determine the best color scheme to set /Settings/Gui/ColorScheme from venus-platform, right? If the auto mode is included in /Settings/Gui/ColorScheme, gui-v2 would need to determine the best color scheme to apply. Venus platform cannot set the actual (light/dark) color scheme then because the color scheme is set to auto.

ReinvdZee commented 9 months ago

@jhofstee, @mpvader, @mansr, regarding the Link Local IP address to be fetched by venus-platform, would the following work for us?

blammit commented 9 months ago

The /Settings/Gui/ColorScheme setting is created by gui-v2:

Note that is just for the mock values backend for testing, and not for dbus/mqtt production mode. gui-v2 doesn't create any of the settings used in production; they are created outside of gui-v2.

@ReinvdZee sorry, my comment about changing the auto color mode earlier was incorrect. It is as you say, that /Settings/Gui/ColorScheme should not be modified by venus-platform, because gui-v2 uses that to show the value that was selected by the user (rather than the auto-color value determined by the system). So, when /Settings/Gui/ColorScheme changes, venus-platform would somehow determine whether the Dark or Light setting should be used, and change /Settings/Gui/AutoColorMode to 0 (Dark) or 1 (Light) as necessary. Then gui-v2 can use /Settings/Gui/AutoColorMode to determine the actual theme to be used, and leave the /Settings/Gui/ColorScheme value on 2 (auto).

ReinvdZee commented 9 months ago

I discussed with @mpvader, the display theme auto mode will be removed from gui-v2 for now.

blammit commented 9 months ago

Thanks for confirming - I will remove it.

[Edit: removed now, in https://github.com/victronenergy/gui-v2/pull/699]

mpvader commented 9 months ago

The /Settings/Gui/ColorScheme setting is created by gui-v2

@ReinvdZee will make venus-platform do it.

Afaik, its impossible for settings to be created over MQTT, and therefore gui-v2 will not create any settings at all., also @jepefe

chriadam commented 8 months ago

This is still blocked waiting on venus-platform. Marking as high_prio. It's definitely for 1.0.0 milestone.

ReinvdZee commented 8 months ago

This is still blocked waiting on venus-platform. Marking as high_prio. It's definitely for 1.0.0 milestone.

~I will take care of adding /Settings/Gui/ColorScheme to venus-platform today.~ -> Please see venus-platform -> guiv2_colorscheme

ReinvdZee commented 8 months ago

Discussed brightness control with @mpvader and @jhofstee today. It seems that the brightness settings used percentages before but this was changed to the absolute value (the actual maximum brightness level, e.g. 15 on the Cerbo). This was changed to accommodate for problems with the brightness slider on the gui (with less than 100 brightness steps, the brightness will not change for every step of the slider).

@blammit, earlier, we agreed:

/Settings/Gui/Brightness : The current brightness setting, in 0 to 100% (let venus-platform convert this setting in 0-100% to the supported brightness range, prevents an unnecessary dbus setting for the max brightness)

To prevent such problems from occuring again, the brightness setting will be in the range of 0 to max brightness, and the slider in the gui should have an equal number of steps. I will change venus-platform to create the setting /Settings/Gui/Brightness with its maximum equal to the maximum system brightness. The maximum can be requested over MQTT so there is no need for an additional dbus setting (e.g. mosquitto_pub -h 10.50.20.56 -t R/48e7da87ec15/settings/0/Settings -m '')

jpetrell commented 8 months ago

Debug: vePlatform.getMemInfo() and QuickView.imageCacheSize() -> ON HOLD BECAUSE OF LOW PRIO

Hid the unfinished memory settings ^^ for now.

ReinvdZee commented 8 months ago

This is done:

I will change venus-platform to create the setting /Settings/Gui/Brightness with its maximum equal to the maximum system brightness.

The setting minimum is set to 1 to prevent the display backlight from being turned off completely by writing 0 to /Settings/Gui/Brightness. The minimum is also reported over MQTT when requested.

edit: I've also removed backlight control from gui-v1.

mpvader commented 8 months ago

@blammit and co,

all changer wrt the display settings are in the latest Venus OS build, v3.30~8.

I think you can finish & test your side now.

DanielMcInnes commented 8 months ago

@blammit - check how gui-v1 adds a delay to prevent the slider jumping

mpvader commented 8 months ago

Wrt the display settings, @blammit , when implementing them in gui-v2, have a look at below commit in gui-v1 and at least test if you need to do something similar in gui-v2:

https://github.com/victronenergy/gui/commit/b75a3dd199092dbd53a7e1d15295f48ed4655ea4

Other comments:

blammit commented 7 months ago

Relays: set relay state based on relay polarity and alarm status (in gui-v1 this is done via the 'Relay' singleton) -> implemented in per Venus OS v3.30~something; so its up for implementation in gui-v2

Since this is now handled in venus-platform, gui-v2 doesn't need to do anything special to add this behaviour. Will mark this part as done in the description.

jpetrell commented 4 months ago

Remote console settings: set remote console password using vePlatform.setRemoteConsolePassword() -> ON HOLD until MQTT security is fixed.

Dropped by MQTT security changes.

"General: Set root password" is the only remaining bit.

chriadam commented 4 months ago

Root password is being covered under #811 so closing this task.

jpetrell commented 4 months ago

Root password is being covered under https://github.com/victronenergy/gui-v2/issues/811 so closing this task.

Implemented in the gui-v2 security-profile branch.