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

feat: add remote modbus rtu driver support, refactor ModbusRTUDriver to use SerialPort directly #1394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flxzt
Copy link

@flxzt flxzt commented May 6, 2024

Description

This PR adds support for using Modbus RTU serial ports remotely. It does that by removing the ModbusRTU resource, and let the driver use SerialPort and NetworkSerialPort resources directly.

In order to archieve this, I moved the fields timeout and address into the driver.

My reasoning is that it makes more sense to let the resource manage the entire bus, and let the driver only control one slave (= address). Same for the timeout, this can be slave-specific.

It seems to be working fine remotely with a Waveshare RTU relay, so far I only tested it with a low baudrate 9600 so I don't know how well the remote serial connection behaves with higher rates.

as mentioned in #1370 I am unsure if it is "okay" to replace the ModbusRTU resource, because this is obviously a breaking change.
Adding a exported NetworkModbusRTU resource would be the alternative, but it would mean a lot of repetition with NetworkSerialPort. But I might have missed intentional architectural decisions here, so I can implement whatever makes the most sense!

Once this is resolved/merged, I'll follow up with an added WaveshareRTURelay driver

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • PR has been tested

refactor modbusrtudriver to use serialport resources directly

Signed-off-by: Felix Zwettler <felix.zwettler@duagon.com>
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

1 participant