Add ASCII/RTU over TCP transports

[andig]

This PR implements ASCII/RTU "on the wire" encoding and transmits it over TCP. This allows to use cheap RS485/Ethernet adapters like USR-TCP232-304 or USR-TCP232-410 without workarounds using external components like pseudo ttys and socat. Apparently those adapters don't speak "Modbus TCP" on the client side, but rather relay "Modbus RTU" verbatim. I cannot tell if this conforms to any modbus standard (see stackoverflow for discussion), but I've verified this is working.

The PR is ported from goburrow/modbus#42 with minor adjustments for logging. It does not use the improved tcpTransporter.send logic as that is specific for TCP encoding as far as I could tell.

Refs snaptec/openWB#290, fixes #20.

[andig]

It would be helpful to enable travis to get immediate feedback on the PR apart from reviews.

[frzifus]

thanks @andig good point. Internally we use buildkite and i will check if this works with external prs.

[guelfey]

@frzifus you can enable it to also build PRs from external contributors, but I'd rather not since it would allow anyone to execute arbitrary code on our nodes. We could instead push the changes to a normal branch after a quick review that the PR doesn't mess with the pipeline

[andig]

Will fix the imports tomorrow. For build pipeline you might want to add travis in addition to what you‘re running internally?

[guelfey]

fine with me, think we have to add Travis as an app to our github org then (@ecktom @jhedev)?

[andig]

Happy New Year! Any news on this one?

[gq0]

@frzifus you can enable it to also build PRs from external contributors, but I'd rather not since it would allow anyone to execute arbitrary code on our nodes. We could instead push the changes to a normal branch after a quick review that the PR doesn't mess with the pipeline

The buildkite setup we continuously us and maintain the configuration. Hence, I prefer an integration branch here to run our CI system on it.

[ecktom]

Hi guys, I'm already working on providing CI for our public repos. Should be in place by end of next week ;)
We are going to use a separate buildkite queue and limited access to the repo's pipeline configuration.

[gq0]

Apparently those adapters don't speak "Modbus TCP" on the client side, but rather relay "Modbus RTU" verbatim. I cannot tell if this conforms to any modbus standard (see stackoverflow for discussion), but I've verified this is working.

These adapters just repackaging serial frames into TCP payload and vice versa. Unfortunately, a Modbus RTU or ASCII frame as TCP payload does not make Modbus TCP. Therefore, what these adapters speak is outside of the Modbus specification and to my opinion does not fit into a Modbus library. But I can understand that you want your setup to run. What speaks against the solution with the virtual serial device, e.g. via socat?

Alternatively, we could make the transport and decoding instances for each combination (serial+RTU/ASCII & TCP+ModbusTCP) accessible to the outside, this way you can create a custom handler.

@frzifus what is your opinion on all this? You worked the most with Modbus.

[andig]

These adapters just repackaging serial frames into TCP payload and vice versa. Unfortunately, a Modbus RTU or ASCII frame as TCP payload does not make Modbus TCP.

Thats true.

Therefore, what these adapters speak is outside of the Modbus specification and to my opinion does not fit into a Modbus library.

As we've seen (also from external comments) this is a very popular configuration, even if not standard.

But I can understand that you want your setup to run. What speaks against the solution with the virtual serial device, e.g. via socat?

It's an external dependency outside goland that would need to be maintained. Especially for dockerized applications anything that moves beyond "single purpose" containers is a nightmare.

Alternatively, we could make the transport and decoding instances for each combination (serial+RTU/ASCII & TCP+ModbusTCP) accessible to the outside, this way you can create a custom handler.

I would be happy to provide and maintain that. However, it feels that the changes required to do so properly are much more invasive than the solution suggested here.

[guelfey]

LGTM, but also fine with splitting it up, up to @gq0 and @frzifus

[andig]

LGTM, but also fine with splitting it up, up to @gq0 and @frzifus

Could I suggest that we proceed as-is? Could still split up later. With go modules you could then even remove the new clients again and declare an incompatible v2 version.

[frzifus]

y, i am fine with that. Basically we should redesign a bit more than that in a upcoming version. 💃

@andig could you adopt the commit messages, please? SEE: #22 (review)

[andig]

could you adopt the commit messages, please?

Done. This is a bit painful, especially with multiple commits and will break following the history due to the needed force-push. May I suggest using squash&commit which allows to adjust the commit message?

[frzifus]

Done. This is a bit painful, especially with multiple commits and will break following the history due to the needed force-push. May I suggest using squash&commit which allows to adjust the commit message?

thanks! Sure, your feedback is always welcome.

here are my thoughts: force push should be no problem as long as it is your own branch/pr. Basically you can also rename single commits without losing the history or breaking thinks. It's good stuff imagine you want to edit e.g. an incomprehensible or simply wrong commit message that you added when you started working on a feature.

SEE: https://gist.github.com/nepsilon/156387acf9e1e72d48fa35c4fabef0b4#not-pushed--old-commit

[andig]

here are my thoughts: force push should be no problem as long as it is your own branch/pr. Basically you can also rename single commits without losing the history or breaking thinks. It's good stuff imagine you want to edit e.g. an incomprehensible or simply wrong commit message that you added when you started working on a feature.

My though was more that commits need to address review findings (like here). Those are typically irrelevant for the merge commit as they show the development process, not the desired result. Renaming won't change that. Force-pushing in the end forces you to rely on me not accidentally sneaking in bad stuff or review again. But anyway, both ways work. Thank you!