search

thiagoralves / OpenPLC_v3

OpenPLC Runtime version 3
GNU General Public License v3.0
1.12k stars 453 forks source link

Fix handling write requests of 64 bit values from modbus #250

Closed Abestanis closed 3 months ago

Abestanis commented 3 months ago

Hello, thank you for this awesome project!

I was trying to write an Int64 value via modbus and noticed that only the first byte seems to be written to memory. After digging around in the source code, I discovered that there is a copy-paste error (from WriteRegister to WriteMultipleRegisters). This PR fixes that copy paste error.

I've run an OpenPLC runtime instance with these changes and can confirm that I am able to correctly write an Int64 value via modbus.

I want to keep the diff easy and simple for you to see the error, but I think the better solution rather than just fixing the copy-paste error would be to extract the common code between WriteRegister and WriteMultipleRegisters, so that bugs like these are less likely to happen. Should I do that?

thiagoralves commented 3 months ago

Thank you for catching that! I should've made better use of my copy/paste ability :)

Abestanis commented 3 months ago

Thank you for catching that!

No problem, thank you again for this awesome project!

I should've made better use of my copy/paste ability :)

Happens to everybody. 😅

Do you want me to extract the common code between WriteRegister and WriteMultipleRegisters into a separate function?

thiagoralves commented 3 months ago

Do you want me to extract the common code between WriteRegister and WriteMultipleRegisters into a separate function?

Sure, that will reduce the complexity of them. Thanks.