DennisOSRM / hms-mqtt-publisher

HMS-XXXXW-2T MQTT publisher and Home Assistant addon
BSD 2-Clause "Simplified" License
111 stars 16 forks source link

Publish inverter ID via simple_mqtt #119

Closed bellackn closed 4 months ago

bellackn commented 5 months ago

I was wondering why this didn't happen for simple_mqtt, but for Home Assistant it did. It makes sense when you want to push data from multiple identical inverters to a single MQTT server, so you can distinguish between them.

bellackn commented 5 months ago

i also have another version running where the inverter ID is part of the published topic and not the payload, which for my personal setup makes more sense. if that's something you'd prefer, i can open another PR and close this one.

dominikandreas commented 5 months ago

I think it would make sense to have the inverter ID as part of the published topic. It would be a breaking change however for the existing users and their applications that depend on the previously used topic. Maybe we can add a parameter "use_sn_in_topic" or something like that to make it optional

bellackn commented 5 months ago

@DennisOSRM already seems to have started to implement reading environment variables in #61 as it looks like. what's the status of this? the fix in this PR here should be done regardless i think, since the home assistant config already does that.

also there is #116 which requests that, basically. let me know if i should pick this up.

DennisOSRM commented 5 months ago

Oh yes, happy to consider PRs since I am somewhat spread thin with day job and other stuff right now. 👍🏽

DennisOSRM commented 4 months ago

I think it would make sense to have the inverter ID as part of the published topic. It would be a breaking change however for the existing users and their applications that depend on the previously used topic. Maybe we can add a parameter "use_sn_in_topic" or something like that to make it optional

I am of the same opinion. It makes sense to have the inverter id as part of the topic.

bellackn commented 4 months ago

alright. closing this one then.