search

celsworth / lxp-bridge

A bridge to MQTT/InfluxDB/Postgres for communications with LuxPower inverters
MIT License
9 stars 14 forks source link

Retain holding and parameter registers #154

Closed lupine closed 1 year ago

lupine commented 1 year ago

Builds on top of #143

Home Assistant is much more pleasant to use with these controls built on top of holding register data if the messages are retained. Otherwise, a restart of home assistant results in the fields being set to "unknown" until a restart of lxp-bridge, or use of the control.

celsworth commented 1 year ago

Are we happy that this is always set to true, no configuration option for it?

Not a leading question :) It might well be that it makes sense to make this change permanent and never not retain these.

lupine commented 1 year ago

@celsworth perfectly fine question ^^.

Certainly for me, it either makes sense to retain these registers or it doesn't, and there isn't much value in making it configurable. The way it makes HA more usable is a good argument for them being retained, IMO. Probably the best time to consider adding configuration for it is the point where someone asks for that functionality?

I did have a minor crisis of confidence over registers 12, 13, and 14 - the current time - where the data quickly gets out of date. It feels like perhaps retaining those registers is a bad idea (and perhaps the same for some others?) - but that's a further argument against configuration, I think. If we start making decisions about what's best per-register, and also allow configuration overrides of that, it starts to get complicated quickly.

lupine commented 1 year ago

Assuming we want this, do we want it to be part of 0.10, since it forms part of the same logical block of functionality?

celsworth commented 1 year ago

Ok, lets just go with making them retained and if someone asks for the config we can add it then. Do we want to think about a way to cleanup retained topics when lxp-bridge exits? Just wondering if we should be good MQTT netizens and not leave topics lying around potentially forever.

Registers 12/13/14 are a bit odd, I think they're a special case in that they're the only ones that can change themselves. It seems pretty unlikely anyone would rely on them being up to date in MQTT so I'd be happy with a proviso in the documentation that those can't be used from the retained topics reliably.

And yes, lets get this into v0.10 as it seems like it rounds off the HA functionality nicely.

lupine commented 1 year ago

Do we want to think about a way to cleanup retained topics when lxp-bridge exits? Just wondering if we should be good MQTT netizens and not leave topics lying around potentially forever.

Do we know what that would look like? We do have the overall lxp/LWT topic which, e.g., home assistant will use to effectively ignore these retained messages... do you think that we should have a similar mechanism for each of these retained messages as well?

I'm very much an MQTT n00b so don't know what the correct etiquette is here :sweat_smile:

celsworth commented 1 year ago

Do we know what that would look like? We do have the overall lxp/LWT topic which, e.g., home assistant will use to effectively ignore these retained messages... do you think that we should have a similar mechanism for each of these retained messages as well?

I think its a slightly different problem, the LWT is probably fine to be left hanging around because as you say thats what HA and other things can use to determine whether lxp-bridge is alive or not.

In fairness we probably can't guarantee that we remove the holding topics when exiting as we might be doing it in an uncontrolled manner anyway, panicing, lost connection to MQTT etc. So maybe this is just worrying too much, we can just leave them :)

lupine commented 1 year ago

MQTT 5 would allow us to have an expiry time on the messages, which would be the ideal low-effort solution... but I recall you mentioning that we're pinned to mqtt 3 at the moment.

celsworth commented 1 year ago

I'd not checked for a while, there's a mention of it in a recent rumqttc blog post - https://bytebeam.io/blog/rumqtt-r19-persistence-10k-load-and-public-infra/#🧭-mqtt-5-roadmap

But not sure of the latest progress. Once its available in that crate we could start using it..

celsworth commented 1 year ago

I've added a brief explanatory line into https://github.com/celsworth/lxp-bridge/wiki/MQTT-Publishes about the new retain behaviour. Can probably be fleshed out later, maybe it deserves a mention in the HomeAssistant page too. In fact the HA functionality has grown so much that page probably needs a revamp entirely.

lupine commented 1 year ago

Great, thanks for the merge and docs update!

I've added a short note about publish_individual_holdings and also opened up https://github.com/celsworth/lxp-bridge/issues/158 , which might be worth getting into 0.10.0 as well?

It might be worth having a list of all controls, and the version they were added, on that page too; I'll see if I can put something together.