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

MQTT ha discovery is incomplete with multiple inverters due to nonunique ids #63 #64

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LeighAS
Copy link

@LeighAS LeighAS commented Sep 12, 2022

mqtt inverter serial number update for multiple inverters

made serial a local variable to the function where mqtt messages where being created. Also included inverter serial as part of the mqtt data packet being sent. (not strictly necessary but helped with easy debugging)

made serial a local variable to the function where mqtt messages where being created. Also included inverter serial as part of the mqtt data packet being sent. (not strictly necessary but helped with easy debugging)
Copy link
Collaborator

@nagydavid nagydavid left a comment

Choose a reason for hiding this comment

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

seems good, pls resolve the commented lines :)

eversolar.pl Outdated Show resolved Hide resolved
eversolar.pl Show resolved Hide resolved
my %config_data = (
device => {
identifiers => [
$mqtt_serial,
$mqtt_serial_HA,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why change the variable name?

Copy link
Author

Choose a reason for hiding this comment

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

$mqtt_serial_HA is not a name change as such, its an entirely new local variable to ensure that this one was being used (and correctly assigned) and not the original $mqtt_serial.

I'm still not 100% on whether this is the best way to do this, we shouldn't really need 2 serial variables, this works and it preserved the fidelity of the mqtt publish commands which I didn't want to change.

eversolar.pl Outdated Show resolved Hide resolved
fixing indents, originally working on this in notepad++ and used tabs for the spacing as typically that i what i would have used in c#/c++. it wasnt obvious that the document was indented using spaces in notepad++. rededited in vscode and the issue became clear.
@LeighAS
Copy link
Author

LeighAS commented Sep 21, 2022

done all requested indentation changes. Name thing is something you may wish to delve into.

@LeighAS LeighAS closed this Sep 21, 2022
@LeighAS LeighAS reopened this Sep 21, 2022
@henrykuijpers
Copy link

@nagydavid Any help needed on this one?

@nagydavid
Copy link
Collaborator

yes please, if you have time. I am very busy with my work coding.

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