search

grid-x / modbus

BSD 3-Clause "New" or "Revised" License
78 stars 26 forks source link

Allow sharing transports between packagers by adding Clone method (BC) #70

Open andig opened 1 year ago

andig commented 1 year ago

Fix https://github.com/grid-x/modbus/issues/56

This is an alternative approach. It features much smaller api surface but leaves responsibility up to the user to understand the transporter's shared nature.

Note: decide name. Is Clone() sufficiently clear? Maybe Share() or ShareConnection() or maybe WithPackager?

andig commented 1 year ago

Friendly ping @frzifus. This would be really helpful using the module downstream and cross-goroutines. The diff looks bigger than it is as I've moved the ...Client routines above the respective ...Handlers for clarity.

andig commented 1 year ago

Any chance for feedback on this change?

andig commented 1 year ago

Friendly ping. This PR is needed to make a modbus connection usable from multiple goroutines, one per id.

andig commented 1 year ago

ping @frzifus did you get a chance to review?

andig commented 5 months ago

ping @frzifus any chance to take a look? Imho this really is a smallish change, although the diff looks large.

andig commented 5 months ago

Since users may initialized handlers without the new... functions, I would consider it to mark it as breaking change. wdyt?

That's a good point. Not doing so will be obvious due to nil pointer panics, but it's definitely breaking. Since this is the case, I'd be happy to add a more fluent way of passing parameters, too if that would help to move this PR along.

andig commented 2 months ago

If anybody wants to test this, I've created a fork at https://github.com/evcc-io/modbus. Happy to update this PR if interest arises.

andig commented 2 days ago

While is PR is meanwhile severely bit-rotten and for sake of reference, we've been successfully using this with evcc.io for ~3 months now and are happy to report that we can have concurrent Go routines interacting with Modbus devices now using different slave ids.