simonvetter / modbus

Go modbus stack (client and server)
MIT License
280 stars 89 forks source link

Read signed integers #34

Closed philon123 closed 1 year ago

philon123 commented 1 year ago

Hi, we can currently only read Uint16 and Uint32. Reading Int16 and Int32 would be great to add!

simonvetter commented 1 year ago

Hi Philip,

I'm trying to keep the API simple and concise, so I'd rather not double the number of Read /Write methods with their signed variants. With that said, this is how I usually do it:

// read a 16-bit, signed integer:
var  reg uint16
err, reg = client.ReadRegister(0x1000, modbus.HOLDING_REGISTER)
if err != nil { ... }
fmt.Printf("unsigned value: %v, signed value: %v\n", reg, int16(reg))

// write a 16-bit, signed register:
var setpoint int16
setpoint = -100
err = client.WriteRegister(0x1000, uint16(setpoint))

Would that work for you?

philon123 commented 1 year ago

Hi Simon,

I've just tested it again now. When casting the uint value to int, it keeps the same logical value, but doesn't reinterpret the underlying bits (which is what it needs to do).

The correct code to convert from uint to int is not so nice: vv := *(*int16)(unsafe.Pointer(&v))

And it took my some trial and error to get right (was previously trying to subtract math.MAX_INT16/2 from the uint value..). So, to help out others in the future, I recommend to add this conversion inside your library for int16, int32, and int64 types.

simonvetter commented 1 year ago

When casting the uint value to int, it keeps the same logical value, but doesn't reinterpret the underlying bits (which is what it needs to do).

Wait, I'm confused. Mind showing me some code where casting between uint{16,32,64}<>int{16,32,64} is not giving the expected results?

philon123 commented 1 year ago

oh, I guess you're right! After testing like this, I see that my horrible reinterpret cast is not required at all. https://go.dev/play/p/RZ0FI1bWQCm

I still think it would be nice to add the int* options to your api though, to avoid confusion like mine and for ease of use!

Thanks for the fast feedback

philon123 commented 1 year ago

aha, I found where I got confused. I was trying to case the uint16 value to int, which doesn't work since, I suppose, it automatically chooses int64 to fit a large value? So, to get negative values as int, you need to do int(int16(ux))