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

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

Open
ogurevich opened this issue Jun 2, 2024 · 8 comments
Labels
question Further information is requested

Comments

@ogurevich
Copy link

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
Copy link
Author

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
Copy link
Collaborator

mman commented Jun 3, 2024

@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
Copy link
Author

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
Copy link
Collaborator

mman commented Jun 3, 2024

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
Copy link
Author

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
Copy link
Collaborator

mman commented Jun 3, 2024

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
Copy link
Author

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

@mman
Copy link
Collaborator

mman commented Jun 3, 2024

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...

@mman mman added the question Further information is requested label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants