Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tank config parameters can’t be set in some situations #1899

Closed
mpvader opened this issue Feb 8, 2025 · 10 comments · Fixed by #1913
Closed

tank config parameters can’t be set in some situations #1899

mpvader opened this issue Feb 8, 2025 · 10 comments · Fixed by #1913
Assignees
Labels
bug Something isn't working high_prio High priority
Milestone

Comments

@mpvader
Copy link
Contributor

mpvader commented Feb 8, 2025

Affects low and high alarm levels and averaging time, and perhaps more fields.

Tank capacity seems to be not affected.

Its a bit weird where it does and does not reproduce:

  • v3.54: gui-v2 on the screen (ie the arm build) only. gui-v1 and gui-v2 wasm work ok.
  • v3.60~latest: gui-v1 and gui-v2 on screen. Gui-v2 wasm works ok.

(Updated on 2025-2-13)

@mpvader mpvader added bug Something isn't working high_prio High priority labels Feb 8, 2025
@mpvader mpvader added this to the v3.50-fixes milestone Feb 8, 2025
@chriadam chriadam self-assigned this Feb 10, 2025
@chriadam
Copy link
Contributor

Can this issue be clarified a bit: when you say "tank config parameters can't be set" do you mean that you never get the option to set them (i.e. PageTankSetup.qml doesn't show the "Averaging Time" (/FilterLength) value, OR do you mean that you get the option to set them but when you change the value it doesn't "stick"?

Regardless, some weird stuff is going on. I'll need to talk to @blammit to understand this better, I think.
In my testing (qinetic-cerbogx with boat/motorhome demo1 running) with current main branch build, there are some strange inconsistencies:

  1. When I go to Device List -> Black water tank (1) and click setup (i.e. PageTankSetup.qml) I only see Capacity, Fluid Type, Volume Unit settings which I can modify. I assume that this is because isValid returns false for all of the other paths (e.g. /FilterLength), and so this seems "fine" to me, so far (i.e. this particular tank doesn't support setting a value for FilterLength, so that path doesn't exist, so isValid returns false for it, so we hide that option in the UI).

  2. However: when I go to Debug -> Values -> com.victronenergy.tank.socketcan_can0_di1_uc69 (i.e. PageDebugVeQItems.qml with bindPrefix = dbus/com.victronenergy.tank.socketcan_can0_di1_uc69) this page DOES show a bunch of other paths (with apparently null values), including:

BatteryVoltage		-
ButaneRatio		-
FilterLength		-
RawUnit			-
RawValue		-
RawValueEmpty		-
RawValueFull		-
SenseType		-
Serial			-
Shape			-
Standard		-
Status			-
Temperature		-

and this is just coming from a VeQItemTableModel so I would normally assume those paths must be enumerated from dbus.
BUT when I use dbus-spy to inspect that path, all I see is:

com.victronenergy.tank.socketcan_can0_di1_uc69
Capacity                                                                                                               1
Connected                                                                                                              1
DeviceInstance                                                                                                         1
FirmwareVersion                                                                                                  v3.0.2V
FluidType                                                                                                              5
HardwareVersion                                                                         VE.Can Resistive Tnk Sndr Adpter
Level                                                                                                             87.776
Mgmt/Connection                                                                                                   VE.Can
Mgmt/ProcessName                                                                                              vecan-dbus
Mgmt/ProcessVersion                                                                                                 2.24
N2kUniqueNumber                                                                                                       69
ProductId                                                                                                          41312
ProductName                                                                                                  Tank sensor
Remaining                                                                                                        0.87776
Serial                                                                                                                 -

i.e. none of the null valued paths (except for Serial) are present.
I tried refreshing (with g) and this made no difference.

I also double checked by introspecting with dbus-send directly:

root@einstein:~# dbus-send --system --print-reply --reply-timeout=2000 --type=method_call --dest=com.victronenergy.tank.socketcan_can0_di1_uc69 / org.freedesktop.DBus.Introspectable.Introspect
method return time=1739166578.571911 sender=:1.128 -> destination=:1.232 serial=171 reply_serial=2
   string "<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node name="/">
  <interface name="org.freedesktop.DBus.Introspectable">
    <method name="Introspect">
      <arg direction="out" type="s" />
    </method>
  </interface>
  <interface name="com.victronenergy.BusItem">
    <method name="GetValue">
      <arg direction="out" type="v" />
    </method>
    <method name="GetItems">
      <arg direction="out" type="a{sa{sv}}" />
    </method>
    <signal name="ItemsChanged">
      <arg type="a{sa{sv}}" name="items" />
    </signal>
  </interface>
  <node name="Capacity"/>
  <node name="Connected"/>
  <node name="DeviceInstance"/>
  <node name="FirmwareVersion"/>
  <node name="FluidType"/>
  <node name="HardwareVersion"/>
  <node name="Level"/>
  <node name="Mgmt"/>
  <node name="N2kUniqueNumber"/>
  <node name="ProductId"/>
  <node name="ProductName"/>
  <node name="Remaining"/>
  <node name="Serial"/>
</node>
"

i.e. no FilterLength node exists in the introspection data.

Similarly, calling GetItems didn't return any FilterLength value:

root@einstein:~# dbus-send --system --print-reply --reply-timeout=2000 --type=method_call --dest=com.victronenergy.tank.socketcan_can0_di1_uc69 / com.victronenergy.BusItem.GetItems
method return time=1739166709.504126 sender=:1.128 -> destination=:1.235 serial=174 reply_serial=2
   array [
      dict entry(
         string "/Serial"
         array [
            dict entry(
               string "Value"
               variant                   array [
                  ]
            )
            dict entry(
               string "Text"
               variant                   string ""
            )
         ]
      )
      dict entry(
         string "/Capacity"
         array [
            dict entry(
               string "Value"
               variant                   double 1
            )
            dict entry(
               string "Text"
               variant                   string "1.0000m3"
            )
         ]
      )
      dict entry(
         string "/DeviceInstance"
         array [
            dict entry(
               string "Value"
               variant                   uint32 1
            )
            dict entry(
               string "Text"
               variant                   string "1"
            )
         ]
      )
      dict entry(
         string "/HardwareVersion"
         array [
            dict entry(
               string "Value"
               variant                   string "VE.Can Resistive Tnk Sndr Adpter"
            )
            dict entry(
               string "Text"
               variant                   string "VE.Can Resistive Tnk Sndr Adpter"
            )
         ]
      )
      dict entry(
         string "/N2kUniqueNumber"
         array [
            dict entry(
               string "Value"
               variant                   uint32 69
            )
            dict entry(
               string "Text"
               variant                   string "69"
            )
         ]
      )
      dict entry(
         string "/Mgmt/ProcessVersion"
         array [
            dict entry(
               string "Value"
               variant                   string "2.24"
            )
            dict entry(
               string "Text"
               variant                   string "2.24"
            )
         ]
      )
      dict entry(
         string "/Mgmt/ProcessName"
         array [
            dict entry(
               string "Value"
               variant                   string "vecan-dbus"
            )
            dict entry(
               string "Text"
               variant                   string "vecan-dbus"
            )
         ]
      )
      dict entry(
         string "/Mgmt/Connection"
         array [
            dict entry(
               string "Value"
               variant                   string "VE.Can"
            )
            dict entry(
               string "Text"
               variant                   string "VE.Can"
            )
         ]
      )
      dict entry(
         string "/Level"
         array [
            dict entry(
               string "Value"
               variant                   double 87.776
            )
            dict entry(
               string "Text"
               variant                   string "87.8%"
            )
         ]
      )
      dict entry(
         string "/Connected"
         array [
            dict entry(
               string "Value"
               variant                   uint32 1
            )
            dict entry(
               string "Text"
               variant                   string "1"
            )
         ]
      )
      dict entry(
         string "/Remaining"
         array [
            dict entry(
               string "Value"
               variant                   double 0.87776
            )
            dict entry(
               string "Text"
               variant                   string "0.8778m3"
            )
         ]
      )
      dict entry(
         string "/ProductId"
         array [
            dict entry(
               string "Value"
               variant                   uint16 41312
            )
            dict entry(
               string "Text"
               variant                   string "0xA160"
            )
         ]
      )
      dict entry(
         string "/FirmwareVersion"
         array [
            dict entry(
               string "Value"
               variant                   string "v3.0.2V"
            )
            dict entry(
               string "Text"
               variant                   string "v3.0.2V"
            )
         ]
      )
      dict entry(
         string "/ProductName"
         array [
            dict entry(
               string "Value"
               variant                   string "Tank sensor"
            )
            dict entry(
               string "Text"
               variant                   string "Tank sensor"
            )
         ]
      )
      dict entry(
         string "/FluidType"
         array [
            dict entry(
               string "Value"
               variant                   uint32 5
            )
            dict entry(
               string "Text"
               variant                   string "5"
            )
         ]
      )
   ]

So my question is: how is that debug page getting the FilterLength node / list entry value to show?
How is the VeQItemTableModel enumerating the possible leaf nodes, if not via dbus?

  1. Probably unrelated, but it seems like the Capacity value doesn't support GetMin / GetMax methods.
dbus.exceptions.UnknownMethodException: org.freedesktop.DBus.Error.UnknownMethod: Unknown method: GetMin is not a valid method of interface com.victronenergy.BusItem\n") "com.victronenergy.tank.socketcan_can0_di1_uc69" "/Capacity"
dbus.exceptions.UnknownMethodException: org.freedesktop.DBus.Error.UnknownMethod: Unknown method: GetMax is not a valid method of interface com.victronenergy.BusItem\n") "com.victronenergy.tank.socketcan_can0_di1_uc69" "/Capacity"
  1. Another interesting point is that in the Debug -> Values list, I saw a path like: dbus/com.victronenergy.tank.adc_builtin0_2 but this doesn't seem to actually exist at all. e.g., dbus says:
root@einstein:~# dbus-send --system --print-reply --reply-timeout=2000 --type=method_call --dest=com.victronenergy.tank.adc_builtin0_2 / org.freedesktop.DBus.Introspectable.Introspect
Error org.freedesktop.DBus.Error.ServiceUnknown: The name com.victronenergy.tank.adc_builtin0_2 was not provided by any .service files

It went away after I rebooted the cerbogx, so maybe this was leftover from a previous demo mode selection, and somehow the VeQItemTableModel didn't refresh its values properly?

@mpvader
Copy link
Contributor Author

mpvader commented Feb 11, 2025

Hi @chriadam :

Can this issue be clarified a bit: when you say "tank config parameters can't be set" do you mean that you never get the option to set them (i.e. PageTankSetup.qml doesn't show the "Averaging Time" (/FilterLength) value, OR do you mean that you get the option to set them but when you change the value it doesn't "stick"?

The issue is that it doesn't stick. Its super easy to reproduce, you'll see it immediately (but make sure to do it on an actual device rather than wasm in a browser). The popup that allows changing the value opens, then press + or - to change it, press save, and you'll see that nothing happens. I've seen that happen on the averaging time setting, as well as high and low alarm thresholds. Possibly the same can happen on other fields as well.

@mpvader
Copy link
Contributor Author

mpvader commented Feb 12, 2025

Reproduced by Daniel, by enabling a tank input on a Cerbo or Ekrano.

So to reproduce you need to not only (a) do it on a physical device but also make sure to (b) use a real tank input. As opposed to using the demo mode

@DanielMcInnes
Copy link
Contributor

DanielMcInnes commented Feb 12, 2025

@mpvader I don't think the problem is in gui-v2. I see the same problem in gui-v1. I can't change the 'averaging time' to a new value in gui-v1 either (v3.60~25). I debugged this down to the level of seeing the SetValue call get sent to the dbus connection (in int VeQItemDbus::setValue(const QVariant &value)), and at that stage everything looked normal.

There is a callback function void VeQItemDbus::setValueDone(QDBusPendingCallWatcher *call) that reports errors from the dbus connection. It is reporting: "void VeQItemDbus::setValueDone(QDBusPendingCallWatcher*) call->error().name(): "org.freedesktop.DBus.Error.InvalidArgs""

@mpvader
Copy link
Contributor Author

mpvader commented Feb 12, 2025

Hi that is strange. Especially because it works fine when using gui-v2 over mqtt/wasm.

I’ll get someone from the venus os team to look into this.

@mpvader mpvader changed the title Tank config parameters can’t be set on arm build tank config parameters can’t be set in some situations Feb 13, 2025
@DanielMcInnes
Copy link
Contributor

DanielMcInnes commented Feb 13, 2025

@mpvader FYI I can't reproduce this in gui-v1 v3.54, but I can reproduce the issue in gui-v1 v3.60~25.

For gui-v2, i can reproduce with both v3.54 and v3.60~25.

I'll keep digging

@mansr
Copy link

mansr commented Feb 13, 2025

The canbus tank sensors don't have the filter length and alarm functions. Those entries ought to be hidden in the gui.

@mpvader
Copy link
Contributor Author

mpvader commented Feb 13, 2025

The canbus tank sensors don't have the filter length and alarm functions. Those entries ought to be hidden in the gui.

as discussed elsewhere, thats not the issue. I've reproduced the issue myself, Mans will contact me if he can't.

@DanielMcInnes
Copy link
Contributor

DanielMcInnes commented Feb 13, 2025

Video showing the issue in gui-v1 v3.60~25:
https://github.com/user-attachments/assets/eb983f32-e33a-4627-924e-2f5da46a3c7f

DanielMcInnes added a commit that referenced this issue Feb 17, 2025
Only when using a dbus backend connection, and no decimals are being used
Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 17, 2025
Only when using a dbus backend connection, and with 0 decimals
Fixes #1899
@mansr
Copy link

mansr commented Feb 17, 2025

The problem is that the gui now sends a value of type double which is rejected somewhere. A change similar to the one above for gui-v2 makes the problem go away:

diff --git a/qml/MbSpinBox.qml b/qml/MbSpinBox.qml
index 08d41e2246e2..c0d269721314 100644
--- a/qml/MbSpinBox.qml
+++ b/qml/MbSpinBox.qml
@@ -164,7 +164,7 @@ MbItem {
 
 	function save() {
 		var newValue = spinbox.value
-		item.setValue(spinbox.value)
+		item.setValue(item.decimals ? spinbox.value : Math.floor(spinbox.value))
 		focus = true;
 		spinbox.enabled = false
 		exitEditMode(true, newValue)

Perhaps someone more familiar with the ins and outs of those components can suggest the best way to fix this.

DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
@DanielMcInnes DanielMcInnes linked a pull request Feb 18, 2025 that will close this issue
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
Otherwise, builds fail on windows.
Part of #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
Otherwise, builds fail on windows.
Part of #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
Otherwise, builds fail on windows.
Part of #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
When writing values to dbus, if the underlying dbus type is 'integer', convert
the new value to 'integer' before sending the message to dbus.

Fixes #1899
DanielMcInnes added a commit that referenced this issue Feb 18, 2025
Otherwise, builds fail on windows.
Part of #1899
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high_prio High priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants