From c88ad088e1b8144b00d50869c23ccd3835a27e28 Mon Sep 17 00:00:00 2001 From: Jeremy Lorelli Date: Thu, 22 Jun 2023 16:20:16 -0700 Subject: [PATCH 1/3] FIX: interpret analog input values as int16 Both signed and unsigned representations behave like a signed 16-bit integer. In the case of unsigned, the value is limited to the range 0-32767, the former is limited to -32768-32767. As such, treating the values from the terminal as int16 (and thus sign extending them when assigning to RVAL) is the correct way of doing things. This does not handle the MSB representation properly however. That will require more work down the line if we ever decide to support it. For now, users will need to use the unsigned or signed representations for the data to be correctly read. --- ek9000App/src/devEL3XXX.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ek9000App/src/devEL3XXX.cpp b/ek9000App/src/devEL3XXX.cpp index 0b3cead..64be3c7 100644 --- a/ek9000App/src/devEL3XXX.cpp +++ b/ek9000App/src/devEL3XXX.cpp @@ -131,7 +131,7 @@ struct EL30XXStandardInputPDO_t { uint8_t _r2 : 6; // Last bit in this align is Sync error for EL31XX uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; - uint16_t value; + int16_t value; // Must be signed to accommodate bipolar terminals. Unsigned representation still defines range as 0-32767, so this is safe. }; #pragma pack() @@ -339,7 +339,7 @@ struct EL331XInputPDO_t { uint8_t error : 1; uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; - uint16_t value; + int16_t value; }; struct EL3314_0010_InputPDO_t { @@ -351,7 +351,7 @@ struct EL3314_0010_InputPDO_t { uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; uint8_t padding1 : 7; // Pad it out to byte boundary - uint16_t value; + int16_t value; }; #pragma pack() From 56e60f096e0d35ff3a7379b4957d0a5259cab4fb Mon Sep 17 00:00:00 2001 From: JJL772 Date: Wed, 17 Jul 2024 18:19:12 -0700 Subject: [PATCH 2/3] Improve signedness handling for EL4XXX analog output terminals --- ek9000App/src/devEL4XXX.cpp | 51 ++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/ek9000App/src/devEL4XXX.cpp b/ek9000App/src/devEL4XXX.cpp index 7082150..76f83b7 100644 --- a/ek9000App/src/devEL4XXX.cpp +++ b/ek9000App/src/devEL4XXX.cpp @@ -39,6 +39,10 @@ static long EL40XX_init_record(void* record); static long EL40XX_write_record(void* record); static long EL40XX_linconv(void* precord, int after); +struct EL40XXDpvt_t : public TerminalDpvt_t { + bool sign = false; +}; + struct devEL40XX_t { long num; DEVSUPFUN report; @@ -54,6 +58,11 @@ struct devEL40XX_t { epicsExportAddress(dset, devEL40XX); +// The default representation for all of these terminals is signed. Unsigned may also be set, even for the bipolar terminals that +// may produce a negative value. To retain some level of support for unsigned representation, terminals that have a positive +// output range use uint16_t as the PDO type. Bipolar terminals always use int16_t to support negative values and will behave incorrectly +// if you choose the unsigned (or absolute w/MSB sign) representation. + DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4001); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4002); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4004); @@ -66,23 +75,33 @@ DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4021); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4022); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4024); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4028); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4031); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4032); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4034); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4038); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4031); // EL403X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4032); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4034); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4038); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4102); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4104); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4112); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4114); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4112); // EL411X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4114); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4122); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4132); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4134); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4132); // EL413X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4134); + +static bool isTerminalSigned(int id) { + if (id <= 4039 && id >= 4030) + return true; // EL403X + if (id <= 4119 && id >= 4110) + return true; // EL411X + if (id <= 4139 && id >= 4130) + return true; // EL413X + return false; +} static void EL40XX_WriteCallback(CALLBACK* callback) { void* record = NULL; callbackGetUser(record, callback); aoRecord* pRecord = (aoRecord*)record; - TerminalDpvt_t* dpvt = (TerminalDpvt_t*)pRecord->dpvt; + EL40XXDpvt_t* dpvt = (EL40XXDpvt_t*)pRecord->dpvt; free(callback); int status = 0; @@ -103,9 +122,14 @@ static void EL40XX_WriteCallback(CALLBACK* callback) { } /* Set buffer & do write */ - uint16_t buf = (int16_t)pRecord->rval; + char buf[2]; + if (dpvt->sign) + *(int16_t*)buf = (int16_t)pRecord->rval; + else + *(uint16_t*)buf = (uint16_t)pRecord->rval; + status = dpvt->pterm->doEK9000IO(MODBUS_WRITE_MULTIPLE_REGISTERS, - dpvt->pterm->m_outputStart + (dpvt->channel - 1), &buf, 1); + dpvt->pterm->m_outputStart + (dpvt->channel - 1), (uint16_t*)buf, 1); } /* Check error */ @@ -135,7 +159,7 @@ static long EL40XX_init(int) { static long EL40XX_init_record(void* record) { aoRecord* pRecord = (aoRecord*)record; pRecord->dpvt = util::allocDpvt(); - TerminalDpvt_t* dpvt = (TerminalDpvt_t*)pRecord->dpvt; + EL40XXDpvt_t* dpvt = (EL40XXDpvt_t*)pRecord->dpvt; uint16_t termid = 0; /* Verify terminal */ @@ -164,6 +188,9 @@ static long EL40XX_init_record(void* record) { return 1; } + /* Determine if it's signed or not */ + dpvt->sign = isTerminalSigned(termid); + return 0; } From 829e1d28de7ea28106e86a2e7ff707007a334dbe Mon Sep 17 00:00:00 2001 From: JJL772 Date: Thu, 1 Aug 2024 14:16:50 -0700 Subject: [PATCH 3/3] MNT: clang-format --- ek9000App/src/devEL3XXX.cpp | 3 ++- ek9000App/src/devEL4XXX.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ek9000App/src/devEL3XXX.cpp b/ek9000App/src/devEL3XXX.cpp index 64be3c7..72d5be3 100644 --- a/ek9000App/src/devEL3XXX.cpp +++ b/ek9000App/src/devEL3XXX.cpp @@ -131,7 +131,8 @@ struct EL30XXStandardInputPDO_t { uint8_t _r2 : 6; // Last bit in this align is Sync error for EL31XX uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; - int16_t value; // Must be signed to accommodate bipolar terminals. Unsigned representation still defines range as 0-32767, so this is safe. + int16_t value; // Must be signed to accommodate bipolar terminals. Unsigned representation still defines range as + // 0-32767, so this is safe. }; #pragma pack() diff --git a/ek9000App/src/devEL4XXX.cpp b/ek9000App/src/devEL4XXX.cpp index 76f83b7..b52a5ef 100644 --- a/ek9000App/src/devEL4XXX.cpp +++ b/ek9000App/src/devEL4XXX.cpp @@ -58,10 +58,10 @@ struct devEL40XX_t { epicsExportAddress(dset, devEL40XX); -// The default representation for all of these terminals is signed. Unsigned may also be set, even for the bipolar terminals that -// may produce a negative value. To retain some level of support for unsigned representation, terminals that have a positive -// output range use uint16_t as the PDO type. Bipolar terminals always use int16_t to support negative values and will behave incorrectly -// if you choose the unsigned (or absolute w/MSB sign) representation. +// The default representation for all of these terminals is signed. Unsigned may also be set, even for the bipolar +// terminals that may produce a negative value. To retain some level of support for unsigned representation, terminals +// that have a positive output range use uint16_t as the PDO type. Bipolar terminals always use int16_t to support +// negative values and will behave incorrectly if you choose the unsigned (or absolute w/MSB sign) representation. DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4001); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4002); @@ -75,16 +75,16 @@ DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4021); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4022); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4024); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4028); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4031); // EL403X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4031); // EL403X support negative output values. DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4032); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4034); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4038); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4102); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4104); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4112); // EL411X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4112); // EL411X support negative output values. DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4114); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4122); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4132); // EL413X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4132); // EL413X support negative output values. DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4134); static bool isTerminalSigned(int id) {