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

Publish inverter ID via simple_mqtt #119

Closed

Conversation

bellackn
Copy link
Contributor

@bellackn bellackn commented Apr 9, 2024

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.

  • run cargo clippy and fix all issues
  • run cargo fmt to format all source files

@bellackn
Copy link
Contributor Author

bellackn commented Apr 9, 2024

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

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

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

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

@DennisOSRM
Copy link
Owner

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

alright. closing this one then.

@bellackn bellackn closed this Apr 18, 2024
@bellackn bellackn deleted the feature/publish_inverter_id branch April 18, 2024 16:42
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

Successfully merging this pull request may close these issues.

3 participants