search

sittner / linuxcnc-ethercat

LinuxCNC EtherCAT HAL driver
GNU General Public License v2.0
200 stars 137 forks source link

generic driver - 8/16 bit signed floats require cast before return not to loose the signal #98

Closed plopes9000 closed 6 months ago

plopes9000 commented 3 years ago

Here is the fix:

hal_s32_t lcec_generic_read_s32(uint8_t pd, lcec_generic_pin_t hal_data) { int i, offset; hal_s32_t sval; if (hal_data->pdo_bp == 0 && hal_data->bitOffset == 0) { switch (hal_data->bitLength) { case 8: return EC_READ_S8(&pd[hal_data->pdo_os]); case 16: return EC_READ_S16(&pd[hal_data->pdo_os]); case 32: return EC_READ_S32(&pd[hal_data->pdo_os]); } }

offset = ((hal_data->pdo_os << 3) | (hal_data->pdo_bp & 0x07)) + hal_data->bitOffset; for (sval=0, i=0; i < hal_data->bitLength; i++, offset++) { if (EC_READ_BIT(&pd[offset >> 3], offset & 0x07)) { sval |= (1 << i); } }

switch (hal_data->bitLength) { case 8: return (int8_t)sval; case 16: return (int16_t)sval; }

return sval; }

scottlaird commented 6 months ago

Minor tip (from almost 3 years after you wrote this): enclosing code in three backticks (```) will keep GH from mangling your formatting.

I think this is a real bug; we should be sign-extending 8 and 16 bit values read with an offset. I've patched this into http://github.com/linuxcnc-ethercat/linuxcnc-ethercat; feel free to give it a test if you're still interested.

plopes9000 commented 6 months ago

Minor tip (from almost 3 years after you wrote this): enclosing code in three backticks (```) will keep GH from mangling your formatting.

I think this is a real bug; we should be sign-extending 8 and 16 bit values read with an offset. I've patched this into http://github.com/linuxcnc-ethercat/linuxcnc-ethercat; feel free to give it a test if you're still interested.

Thanks, not in this topic at the moment, but thanks for getting to it.