victronenergy / venus-influx-loader

NodeJS server that takes from MQTT into Influx, and config UI and still more
MIT License
12 stars 4 forks source link

Update KeepAlive Publish Path Following Transition from Mosquitto to FlashMQ #108

Open ogurevich opened 3 weeks ago

ogurevich commented 3 weeks ago

According to the documentation in https://github.com/victronenergy/dbus-flashmq?tab=readme-ov-file#keep-alive, the Loader should publish keepAliveOption in a new location in the latest version of Venus OS.

Loader.prototype.sendKeepAlive = function (client, portalId, isFirstKeepAliveRequest) {
  this.logger.debug(`sending keep alive for ${portalId}, isFirstKeepAliveRequest: ${isFirstKeepAliveRequest}`)
  client.publish(`R/${portalId}/keepalive`, isFirstKeepAliveRequest ? '' : '{ "keepalive-options" : ["suppress-republish"] }')
}

To ensure compatibility with older versions of Venus OS, the Loader could either be parameterized, or if possible, the Venus version could be retrieved directly from the MQTT broker. This would ensure that the keepAliveOption is published correctly according to the specific OS version.

ogurevich commented 3 weeks ago

With a 'keepalive-options': ['suppress-republish'] (Venus OS 3.32 - FlashMQ), after a couple of hours, the loader does not receive some values anymore, e.g., Battery/0/Info/MaxChargeVoltage. Without this option, everything worked fine for me. I am not deep enough into the subject matter; could you please take a look at it?

Loader.prototype.sendKeepAlive = function (client, portalId, isFirstKeepAliveRequest) {
  this.logger.debug(`sending keep alive for ${portalId}`);
  client.publish(`R/${portalId}/keepalive`, '');
}

I am not deep enough into the subject matter; could you please take a look at it?

mman commented 3 weeks ago

@ogurevich Thanks for helping me test this. Using the old DBus path should be OK, it still works, and suppress republish should only send changed values. Before the logic was broken in that it requested refresh of everything every 62 seconds. We are trying to get to the stage where only changed values are stored into the InfluxDB in that we really record what really happened without any duplicates. So the question is whether the Battery/0/Info/MaxChargeVoltage should change for you.

ogurevich commented 3 weeks ago

I've been using the Victron-Influx loader with a configuration set to suppress republishing ('keepalive-options': ['suppress-republish']). My understanding was that the loader should log the state of the system in a way that allows for snapshot-like analysis later on. However, with the suppress-republish option, I've observed that the loader stops receiving some values after a few hours, such as SELECT mean("value") FROM "battery/Info/MaxChargeVoltage" WHERE ("instanceNumber"::tag = '2') AND $timeFilter GROUP BY time($__interval) fill(null) This is not ideal as it seems to log points relatively infrequently, as shown in the graph to the left of the vertical line, potentially missing critical changes in the data.

The intended behavior, as I understand it, was to ensure the loader records system states accurately for later analysis, capturing what really happened without duplicates. However, the current setup with suppress-republish may not support this, as it results in less frequent data points, limiting the effectiveness of any subsequent analysis based on these snapshots.

Could you advise on how we might adjust the loader or its configuration to better capture continuous state changes without missing significant data points? image

mman commented 3 weeks ago

The point is to never loose any data. What gets published by the Venus on DBus gets later recorded via MQTT and stored to Influx. So we must not, and we should not be loosing any data.

Another point is how is Influx helping us or not to properly visualise the data. In other words, what would mean actually report for infrequently changing values...

ogurevich commented 3 weeks ago

i understand and agree. on the other hand if the user of dashboard selects time range in this example from 2024-06-02 01:10:26 to 2024-06-02 08:03:07 so he will get no Data Visualisation

Screenshot 2024-06-03 at 18 55 26

On VRM Site for the same time range the Visualisation works fine

Screenshot 2024-06-03 at 19 01 44

we don't store unnecessary duplicated values, but user experience get worse with this strategy.

mman commented 3 weeks ago

Yeah, I understand what you are saying. Let's give it some thought to figure out what is the correct answer in that case.

One of the goals of this project is to get super high granularity data for investigation, and by suppressing re-sends of old values we also store only valid data that were published by the system. So missing data also has a meaning...

Goal of Grafana is to not duplicate VRM functionality, but rather enhance it.

Let's think about what should success look like in this case...

ogurevich commented 3 weeks ago

thank you for clarification and infos, should we close the issue ? And let some time for this thought :)

mman commented 3 weeks ago

thank you for clarification and infos, should we close the issue ? And let some time for this thought :)

No please keep it open, you are raising a valid point and we should figure out what is the proper functionality...