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.

Sign up for GitHub

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

Jump to bottom

Modbus: add UDP and allow concurrent access #13676

Merged
merged 27 commits into from
Aug 2, 2024
Merged

Modbus: add UDP and allow concurrent access #13676

merged 27 commits into from
Aug 2, 2024

Conversation

andig
Copy link
Member

@andig andig commented May 1, 2024 •
edited
Loading

This PR splits logical connection per SlaveID. Underlying physical connection is shared. Fixes #15196.

TODO

  • Make per-connection parameters timeout and delay non-racy. May require refactoring mbmd or removing mbmd dependency at the cost of not being able to share the connection with MBMD devices.

@andig andig added the enhancement New feature or request label May 1, 2024
@andig andig mentioned this pull request May 2, 2024
Closed
@andig andig marked this pull request as draft May 4, 2024 12:46
@github-actions github-actions bot added the stale Outdated and ready to close label Jun 14, 2024
@github-actions github-actions bot closed this Jun 20, 2024
@andig andig reopened this Jun 20, 2024
@andig andig added backlog Things to do later and removed enhancement New feature or request stale Outdated and ready to close labels Jun 20, 2024
@andig
Copy link
Member Author

andig commented Jul 6, 2024

Das verbleibende Problem hier ist, dass die grix-x/modbus API auf structs beruht, wir aber versuchen nach Initialisierung nochmals die Werte zu verändern. Das ist racy.

Lösungen wären entweder:

  • grix-x/modbus API komplett anpassen
  • erst alle Geräte initialisieren und dann dann nutzen

Letzteres ist nicht möglich, da a) bei Initialisierung tw. schon gelesen wird und b) Heartbeats gestartet.

Bliebe noch akzeptieren. Die Auswirkungen sollten verschmerzbar sein, ein Test ob evcc races enthält wäre dann aber nicht mehr möglich.

@andig andig marked this pull request as ready for review July 6, 2024 17:06
oliverheit
oliverheit approved these changes Jul 26, 2024
View reviewed changes
Copy link

@oliverheit oliverheit left a comment

Choose a reason for hiding this comment

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

Ich habe mir alles durchgeschaut und es sieht gut aus.
Ich konnte keine Probleme entdecken.
Die Nutzung des Mutex ist gut integriert.

Mit freundlichen Grüßen
Oliver

@andig
Copy link
Member Author

andig commented Jul 27, 2024

Started merging upstream

@andig andig mentioned this pull request Aug 2, 2024
Closed
@andig andig merged commit 03f62e5 into master Aug 2, 2024
6 checks passed
@andig andig deleted the chore/modbus-udp branch August 2, 2024 19:32
@andig andig mentioned this pull request Aug 6, 2024
Merged
andig added a commit that referenced this pull request Aug 8, 2024
@andig
Revert "Modbus: add UDP and allow concurrent access (#13676)"
de9f5bf 
This reverts commit 03f62e5.
This was referenced Aug 8, 2024
Closed
Merged
@KalkiDE KalkiDE mentioned this pull request Aug 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliverheit oliverheit oliverheit approved these changes

Assignees
No one assigned
Labels
backlog Things to do later
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

wrong charge power for inactive loadpoint
2 participants
@andig @oliverheit