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

[BUG] Adding Virtual Devices causes duplicate device instances #216

Closed
morphias2004 opened this issue Feb 22, 2025 · 6 comments
Closed

[BUG] Adding Virtual Devices causes duplicate device instances #216

morphias2004 opened this issue Feb 22, 2025 · 6 comments

Comments

@morphias2004
Copy link

Describe the bug
I am using Signal K and Node Red to work around the inability of Venus OS to natively handle multiple fluid levels from a single function. The same appears to be true for temperatures.

By using Victron Virtual Devices in Node Red, I can use the data sources that Signal K can see on the NMEA 2000 network.

e.g.

Image

Image

Image

This has raised a new issue in that, when creating the Virtual Devices, they are allocated a device instance starting at 100, with no respect for existing device instances already in use. There does not appear to be a way to manually edit the device instance or a setting to set what number is will start allocating from.

The result is duplicate device instances and sensor data not aligning with sensor names. The existing sensor has it's name overwritten on VRM with the Virtual Sensor name, but the original sensors data is displayed, but the new Virtual Sensor has no data at all.

To Reproduce
Steps to reproduce the behavior:

  1. Have an existing sensor of any type (e.g. temp sensor) set up on a GX device on RPi running Venus OS. It will have a device instance of 100.
    Create a virtual sensor in Node Red of the same type and deploy it.
    The new virtual sensor will be allocated a device instance of 100 - a duplicate of the real sensor.

Expected behavior
A unique device instance to be allocated to Virtual Devices.

Maybe start them at 150 or provide a setting allowing users to set the number they will start at? That and/or give users the ability to manually change the device instance of the Virtual Device.

Screenshots

Image

Image

Image

Image

Image

RPi CPU correctly displaying in VRM:

Image

Image

RPI CPU name overwritten by Virtual Device:

Image

Image

I found I could just add more ‘dummy’ temp sensors till I got past the device instance numbers that were already in use, Deploy, delete the ‘dummies’ and Deploy again. A reboot of Venus OS is required to clear out the ‘dummies’.

Image

Hardware (please complete the following information):

  • RPi4B 4GB

Software (please complete the following information):

  • node-red-contrib-victron version: 3.1.15 App installed via Signal K Appstore. Node Red is disabled in Venus OS Large Features.

Image

  • Signal K 2.11.0
  • Venus OS version: 3.54 Large
@morphias2004
Copy link
Author

Just discovered I can edit the Device Instances in the new v2 GUI web interface:

Image

Image

@dirkjanfaber
Copy link
Collaborator

The proper way of getting the device instance is to first do an addSettings dbus call, which is what the virtual devices do. (See here)

The system ask for example a device instance for ${config.device}, which becomes tank:100. Then the virtual device queries back what actual device instance the Venus system decided to give the device and adjusts it before exporting it to the dbus.

I don't think it is a bug on the node-red-contrib-victron side. Apparently the existing devices just take the number without properly calling the addSettings first to properly register it.

@morphias2004
Copy link
Author

morphias2004 commented Feb 22, 2025

@dirkjanfaber thanks for the clarification.

So does this make it a 'bug' on the Venus OS side? That added sensors are not being properly registered when they are connected?

I have tested changing the device instance in the new v2 GUI, and the change does flow back to Node Red; so that's good.

Either way, I guess that means this thread should be changed to a feature request to have an additional field added to the Virtual Device so that it can be manually set at creation or changed post creation. I think starting at a higher integer would solve the issue as it is unlikely for a system to have more than say 20-30 external inputs.

@dirkjanfaber
Copy link
Collaborator

I still prefer to find the actual cause (even if it is in Venus) and properly fix it there. In order to trace this on the Venus side, I would like to know a little more on the actual sensors you are adding to the system. The adc builtin temperature sensors on GX devices typically have device instances in the 20-30 range.

Feel free to change the number 100 on https://github.com/victronenergy/node-red-contrib-victron/blob/master/src/nodes/victron-virtual.js#L412 on your installation. This is where the starting number for the virtual devices is defined.

@morphias2004
Copy link
Author

Cool. There two temp sensors.

One is the RPi CPU temp, which comes in via this add-in:

https://github.com/TimD1981/RpiTemperature

The other is a DS18B20 1-Wire sensor, which is brought in via this add-in:

https://github.com/Rikkert-RS/VenusOS-TemperatureService

I plan to add a BME280 with this add-in:

https://github.com/pulquero/dbus-i2c

I've made the change in the link you provided. I've never done a Pull Request, so hopefully I've done it correctly.

@dirkjanfaber
Copy link
Collaborator

The first package takes number 6 (https://github.com/TimD1981/RpiTemperature/blob/main/RpiTemperature.py#L203).

The clash in device instance 100 seems to come from https://github.com/Rikkert-RS/VenusOS-TemperatureService, where number 100 is claimed without properly registering it before usage. This is done in this line: https://github.com/Rikkert-RS/VenusOS-TemperatureService/blob/main/dbus-i2c.py#L171.

The BME280 sensors does some semi-random foo to come up with an device instance (https://github.com/pulquero/dbus-i2c/blob/5bd7762b4b63870f6a6ee0809aa3df199f88a67d/service_utils.py#L46).

So all of them just pick a number and hoping that they are the first one to take it, with no plan B. Let alone use the proper way of first calling addSettings and then use the number you are allowed to use.

Feel free to file a bug report in the other repositories, referring to this issue, convincing them to start using addSettings before claiming a number.

I'll close this one as it the node-red-contrib-victron package registers it as it should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants