Implement Modbus UDP

[StefanNienhuis]

This PR implements Modbus RTU over UDP.

Also implements handling of UDP addresses in Modbus CLI and documents this usage in the README.

This solves #50.

[dammarco]

Thanks for the contributin @StefanNienhuis ! 😊

I am not deep into the UDP protocol itself and also not into the implementation here in the repo.
As we have no internal use of Modbus RTU via UDP, I also don't see an issue against adding it to the repo here.

I've added a few comments, nothing critical here

[StefanNienhuis]

@tretmar Thanks for the review. I've made the changes.

[dammarco]

lgtm 👍

[andig]

What's missing to get this merged?

[andig]

@StefanNienhuis after looking through the code (and not being a network protocol guy) I have a situation here in https://github.com/evcc-io/evcc/blob/master/meter/goodwe/server.go where we're sending UDP requests to a fixed port and the device replies on a fixed port:

// receiver
addr, err := net.ResolveUDPAddr("udp", "0.0.0.0:8899")
if err != nil {
	return nil, err
}

instance.conn, err = net.ListenUDP("udp", addr)
if err != nil {
	return nil, err
}

// sender
addr, err := net.ResolveUDPAddr("udp", net.JoinHostPort(ip, "8899"))
if err == nil {
        // hand-built RTU
	_, err = m.conn.WriteToUDP([]byte{0xF7, 0x03, 0x89, 0x1C, 0x00, 0x7D, 0x7A, 0xE7}, addr)
}

// listener
buf := make([]byte, 1024)
n, addr, err := m.conn.ReadFromUDP(buf)
if err != nil {
	continue
}

Do you see any chance to support such a scenario? Or maybe I misunderstand this PR or UDP in general?

[StefanNienhuis]

@andig I’m not quite sure what your goal is, but I see it’s intended for Goodwe inverters, which is exactly what I built this for. The only thing that would be missing is supporting the AA55 prefix it sends before responses on some models, but that may not be required for your model.

[andig]

I think I'm confused by the fact that your code does not have a UDP listener, but I guess I'm just over-complicating UDP mentally.

[StefanNienhuis]

Go abstracts away large parts of the UDP protocol details in their network implementation. This allows you to have a UDP ‘connection’ that can send and receive data similar to TCP, even though there isn’t a persistent connection.

[dammarco]

Sorry, it is common for us that the authors themselves are merging the PR but I guess you either are not allowed or just not think about merging your own PRs 🤔

As this has no impact for us, I will merge it now :)

[andig]

@StefanNienhuis confirmed working in evcc-io/evcc#12752 (reply in thread). Unfortunately we're now confronted with #85 (comment). Seems forking would be the right way to go to implement a workaround for this device defect.

[StefanNienhuis]

Yeah, although it's pretty trivial to implement, it may be out of scope for this project to implement such device specific behavior. Not sure who decides what goes in this project and what not, but since you merged this PR, what do you think @tretmar?

In case you're not familiar, some families of Goodwe inverters accept their commands in normal Modbus format, but send their response in Modbus format with the bytes AA55 prefixed.

[dammarco]

I'd say that device specific behavior doesn't belong into this project here 🤔

Forking brings the risk of getting out of sync really quickly. Another idea is to create an option that you can configure that can deal with the response prefix of AA55. And only use it for your project then.

[andig]

Sorry, it is common for us that the authors themselves are merging the PR but I guess you either are not allowed or just not think about merging your own PRs 🤔

@tretmar nobody contributing here has that permission. See #45 (comment), three years old. There's still a queue of PRs that need attention like the important #81.

Forking brings the risk of getting out of sync really quickly.

Not responding brings the risk of forking.

Another idea is to create an option that you can configure that can deal with the response prefix of AA55. And only use it for your project then.

How would I do that without a fork or a merged PR?

[dammarco]

How would I do that without a fork or a merged PR?

Well, either forking or merged PR ofc. Sure, if you face the issue that you have to wait multiple months to get it merged, I understand your concerns.

[RTTTC]

Sorry, it is common for us that the authors themselves are merging the PR but I guess you either are not allowed or just not think about merging your own PRs 🤔

@dammarco - May I ask how have you became a contributor with rights to merge in this project ?
I think it would beneficial for all of us if @andig got similar rights to merge here.
@gq0 - ?

[andig]

@dammarco works for gridx. No magic.

I think it would beneficial for all of us if @andig got similar rights to merge here.

Its perfectly fine for maintainers to do that.

[andig]

@StefanNienhuis thank you for adding ModbusUDP. I've noticed that the current implementation does not use any timeouts. This can lead to indefinite wait times, at least in my tests. It would be great, if similar timeout to the TCP transporter could be implemented, both for reading and writing.