victronenergy / node-red-contrib-victron

MIT License
87 stars 18 forks source link

[BUG] Custom Control Node doesn't accept string input for string values #191

Closed baroldgene closed 2 months ago

baroldgene commented 3 months ago

Describe the bug If attempting to update DBus settings that are string values (e.g. TimeZone) the Custom Control Node rejects text input with the error "value is not of type number". I believe that this is DBus rejecting the attempt to save since the data sent and the type passed along with it don't match.

To Reproduce Steps to reproduce the behavior:

  1. Using node 'Custom Input Node'
  2. Fetch com.victron.settings with the path "Settings/System/TimeZone"
  3. Route the output directly to Custom Control Node with the same Input and Measurement.
  4. Error message: "Error: Data: America/Denver was not of type number"

Expected behavior I would expect to be able to properly set this setting.

Screenshots

Screenshot 2024-04-05 at 5 40 46 PM Screenshot 2024-04-05 at 5 40 55 PM

Flow If applicable, add a flow to help explain your problem.

Hardware (please complete the following information):

Software (please complete the following information):

Additional context As best I can tell the problem lies in this function. I believe adding the following code after line 302 would solve the issue: if (typeof value == "string" && !isNaN(value)) { numType = 's' }

baroldgene commented 3 months ago

I was attempting to test this fix myself but was unable to due to:

  1. Inability to install a custom version of the node module on the built-in NodeRed install
  2. Complete unfamiliarity with how Node Red modules work
dirkjanfaber commented 3 months ago

Thanks for reporting. I can reproduce it and it indeed needs a fix like the one you proposed. If you want to test it on your Cerbo before the fix will be part of Venus, you can do that as follows:

Anyway, I've prepared the patch and will make a new release early next week and make it part of the next Beta Venus release.

I'll leave this issue open until that has been done.

baroldgene commented 3 months ago

Thanks for the patch and for the help on how to test. I was able to test this on my Cerbo GX and it worked perfectly! Much appreciated!

dirkjanfaber commented 2 months ago

Closing this issue; new release has been created and put on the queue for inclusion into the next Venus beta release.