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

Templates: Rework Modbus config #8600

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from
Draft

Templates: Rework Modbus config #8600

wants to merge 31 commits into from

Conversation

andig
Copy link
Member

@andig andig commented Jun 23, 2023 •
edited
Loading

@premultiply falls Du Lust hast: könntest Du das mal mit einer echten alten Config als auch mit configure testen?

TODO:

  • review Fox
  • translate modbus doc comments
  • review full diff
  • test if configure still works @premultiply
  • Fix double tcp
  • add unit tests for old and new config @andig

@andig andig marked this pull request as draft June 23, 2023 18:43
@premultiply premultiply changed the title Modbus: support ModbusTCP via serial adapter Templates: Add Modbus TCP config for native serial devices Jun 23, 2023
@premultiply
Copy link
Member

Der Titel war verwirrend 😉

@premultiply
Copy link
Member

modbus: tcp | rtu-over-tcp | rtu ?

und dann für choice die Varianten modbus-serial und modbus-net? Vielleicht auch abgekürzt.

@andig
Copy link
Member Author

andig commented Jun 23, 2023

Können wir gerne so umbenennen. Aber funktioniert es denn? Und wie machen wir dem Anwender bei configure klar welche Optionen er da hat? Wie beschreiben wir die Varianten?

@andig andig added the enhancement New feature or request label Jul 3, 2023
@andig
Copy link
Member Author

andig commented Jul 3, 2023

@premultiply warst du hier soweit glücklich?

@premultiply
Copy link
Member

Ja, von meiner Seite konzeptionell schon.
Schau gerne aber nochmal ob du es so gut/besser findest und ob es keine Breaking Changes für den Bestand verursacht.

@premultiply
Copy link
Member

Kommen wir hier weiter?
Ich stoplere da momentan bei nahezu jedem neuen Template drüber.

@andig
Copy link
Member Author

andig commented Jul 23, 2023

Da geht noch was, aktuell hänge ich am config ui

@github-actions github-actions bot added the stale Outdated and ready to close label Aug 13, 2023
@andig andig added backlog Things to do later and removed stale Outdated and ready to close labels Aug 13, 2023
@andig andig changed the title Templates: Add Modbus TCP config for native serial devices Templates: add Modbus TCP config for native serial devices Sep 14, 2023
@premultiply
Copy link
Member

ToDo: Doku-Ausgabe auf Englisch übersetzen

@premultiply premultiply self-assigned this Sep 14, 2023
@premultiply premultiply changed the title Templates: add Modbus TCP config for native serial devices Templates: Rework Modbus config Oct 1, 2023
@premultiply
Copy link
Member

Es gibt noch ein kleines Schönheitsproblem:
Sowohl in configure als auch in der Doku taucht bei Geräten die sowohl eine serielle Modbus RTU- als auch eine native Modbus TCP-Schnittstelle haben Modbus TCP zweimal in der Auswahl auf.

@premultiply premultiply assigned andig and unassigned premultiply Oct 1, 2023
@andig
Copy link
Member Author

andig commented Oct 1, 2023

Ohne die Ursache zu sehen: tatsächlich sind das ja auch 2 unterschiedliche physische Anschlüsse. Auf einem würde ich RTU konfigurieren wollen, auf dem anderen immer Modbus TCP sprechen. Welches Handling stellst du dir vor?

@premultiply
Copy link
Member

Neben der nativen Modbus TCP Schnittstelle kann auch der serielle Port via Konverter Modbus TCP sprechen. Daher die Doppelung in diesem Fall.

@andig
Copy link
Member Author

andig commented Oct 1, 2023

Is klar. Aber was möchtest du erreichen?

@premultiply
Copy link
Member

Vielleicht kann man die doppelte Ausgabe irgendwie mit einfachen Mitteln auf eine reduzieren?

@andig
Copy link
Member Author

andig commented Oct 2, 2023

Eigentlich sind das in dem Fall ja zwei unterschiedliche Geräte. Ich schau mir das mal in configure an.

@andig andig removed the enhancement New feature or request label Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@premultiply premultiply premultiply left review comments

Assignees

@andig andig

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

Successfully merging this pull request may close these issues.
2 participants
@andig @premultiply