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

allow to set client id of mqtt connection in config, to support multi… #52

Merged
merged 3 commits into from
Jul 7, 2022

Conversation

dhavlik
Copy link
Contributor

@dhavlik dhavlik commented May 26, 2022

…ple instances of sungather

@bohdan-s
Copy link
Owner

Thanks!
I have 2 points:

  1. client_id = self.mqtt_config['client_id'] should be self.client_id = self.mqtt_config['client_id']
  2. Do you think the default should be SunGather_[Serial] or SunGather_[Model]? That way its unique even if you dont set it?

@dhavlik
Copy link
Contributor Author

dhavlik commented May 29, 2022

  • client_id = self.mqtt_config['client_id'] should be self.client_id = self.mqtt_config['client_id']

Not really. client_id is only used in the line below.

  • Do you think the default should be SunGather_[Serial] or SunGather_[Model]? That way its unique even if you dont set it?

This is a good idea, I changed it accordingly (using the model - this way it would still require to set manually if you monitor 2 inverters of the same model).
For security reasons, I would not use the serial number for such things (client id will be logged by the mqtt server).

Copy link
Owner

@bohdan-s bohdan-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this commit :) will make it easier for people with multiple inverters.

@bohdan-s bohdan-s merged commit d4c36af into bohdan-s:main Jul 7, 2022
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.

None yet

2 participants