CAN mapping fixes (#18)

* Enable signed gains when mapping via SDO

It is possible with the serial terminal to
configure negative gain parameters for a can
rx or tx map entry. When reading back via
CAN SDO the map entry's gain value is
incorrect. When adding a can map entry via
SDO the gain is interpreted as a positive value
only.

Tests:
 - Create can map entries via CAN SDO using
   openinverter_can_tool with a mix of :
    - gain values -8388.608, -1.0, 1.0 and 8388.607
    - offset values -128, 0 and 127
 - verify "oic can list" shows the map created
 - verify via serial terminal that "can p" shows
    the same can map
 - check can map operation transmits the
    expected behaviour for a negative gain.

* Improve validation when adding can map entries

Validate that the offset and length for a can
map entry will no overflow the resulting CAN
frame. So offset 63/length 1 is valid as is
offset 32/length 32 but offset 60/length 8 is
not. This change affects both serial and SDO.

The length field must be at least 1 bit to make
sense. This was already validated by the SDO
configuration but ignored by serial.

Tests:
 - Verify config of map entries with serial
    terminal fails as documented
 - Verify with a doctored
   openinverter_can_tool without client side
   validation of offset and length that failure is
   reported

* Send mapped params which overlap first/second words

It is possible to configure a CAN map entry
which spans the internal 32-bit word
boundary used to store can frames. When
sending params to CAN only the bits in the
first 32-bit word are actually sent.

The fix involves detecting the overlap
condition and shifting to the two words. It
uses uint32_t consistently to avoid errors due
to sign-extension when shifting.

The receive path already handled the split
word case.

Tests:
 - Create 16, 24 and 32-bit parameter
    mappings at positions that overlap the 32-
    bit boundary.
 - Use positive and negative values in the
    mapped param
 - Verify with a custom DBC in SavvyCAN that
    the values are interpreted and scales
    appropriately.
- Additionally, add mappings either side of the
   split mapping to verify no data overwrite

* Fix parameters mapped to first 32-bits

Correctly map params of 32-bit in size that fit
entirely within the first word of the CAN
frame.

This fixes an integer overflow in the mask
generation which doesn't matter for 32-bit
ARM but is undefined behaviour and causes
problems when compiled for x86-64.

Tests:
 - Map a negative and positive valued params
    to offset 0, length 32 and verify the CAN
    frame contains the expected value in the
    first 4 bytes.

* Fix validation of big-endian CAN map offsets

In big-endian can mappings the offset
specifies the end bit for the mapping and the
negative length counts bits back. The existing
offset validation only worked for little endian
mappings where the values are all positive.
This changes separates the validation.

Tests:
 - Verify 32-bit mappings work with offsets of
   31, 55, 63
 - Verify 32-bit mappings fail with offsets of 7,
   30 and 64

* Implement big-endian tx can mapping

Big-endian transmit can mappings share the same masking as
little-endian mappings. The bytes are then swapped before
shifting into the appropirate position. The maths is different as
the offset position in big-endian mappings specifies the end bit
offset wheras little-endian it's the start bit.

Tests:
 - Create 8-bit mappings with offset 7 and 39
 - Create 16-bit mappings with offset 23, 39 and 47
 - Create 24-bit mappings with offset 23, 55 and 63
 - Create 32-bit mappings with offset 31, 39, 47, 55, 63
 - Test each mapping with positive and negative values
 - Verify the output in SavvyCAN with equivalent Message/Signal

* Fix CAN rx mapping for negative numbers

Negative numbers are not received properly for can rx maps smaller than 32-bits. To fix this we need to sign extend values so they fit in an int32_t before converting to a float. The sign extension makes no sense for single bits as this would result in -1 being returned rather than 1 when the bit is asserted.

Tests:
 - Create BE and LE 8-bit, 16-bit, 24-bit, 31-bit and
    32-bit mappings
 - Verify with positive and negative numbers that
    SavvyCAN reports the expected value

src/canmap.cpp

@@ -80,7 +80,6 @@ bool CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
          uint32_t word;
          uint8_t pos = curPos->offsetBits;
          uint8_t numBits = ABS(curPos->numBits);
-         uint32_t mask = (1 << numBits) - 1;
 
          if (curPos->numBits < 0) //negative length is big endian
          {
@@ -102,7 +101,7 @@ bool CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
             }
 
             //Swap byte order
-            uint8_t* bptr = (uint8_t*)&word;
+            const uint8_t* bptr = (uint8_t*)&word;
             word = (bptr[0] << 24) | (bptr[1] << 16) | (bptr[2] << 8) | bptr[3];
             pos = 31 - pos;
          }
@@ -113,7 +112,7 @@ bool CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
                word = data[1];
                pos -= 32; //position in second word
             }
-            else if ((curPos->offsetBits + curPos->numBits) < 32) //all data in first word
+            else if ((curPos->offsetBits + curPos->numBits) <= 32) //all data in first word
             {
                word = data[0];
             }
@@ -125,7 +124,24 @@ bool CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
             }
          }
 
-         val = (word >> pos) & mask;
+         // extract the bits we are looking for
+         uint32_t mask = (1L << numBits) - 1;
+         word = (word >> pos) & mask;
+
+         // sign-extend our arbitrary sized integer out to 32-bits but only if
+         // it is bigger than a single bit
+         int32_t ival;
+         if (numBits > 1)
+         {
+            uint32_t sign_bit = 1L << (numBits - 1);
+            ival = static_cast<int32_t>(((word + sign_bit) & mask)) - sign_bit;
+         }
+         else
+         {
+            ival = word;
+         }
+
+         val = ival;
          val += curPos->offset;
          val *= curPos->gain;
 
@@ -165,16 +181,50 @@ void CanMap::SendAll()
 
          val *= curPos->gain;
          val += curPos->offset;
-         int ival = val;
-         ival &= ((1 << curPos->numBits) - 1);
+         // convert to a signed integer value before storing in an unsigned to
+         // avoid sign-extension problems when we start shifting and masking
+         uint32_t ival = (int32_t)val;
+         uint8_t numBits = ABS(curPos->numBits);
+         ival &= (1UL << numBits) - 1;
 
-         if (curPos->offsetBits > 31)
+         if (curPos->numBits < 0) // big-endian
          {
-            data[1] |= ival << (curPos->offsetBits - 32);
+            //Swap byte order
+            const uint8_t* bptr = (uint8_t*)&ival;
+            ival = (bptr[0] << 24) | (bptr[1] << 16) | (bptr[2] << 8) | bptr[3];
+
+            if (curPos->offsetBits < 32) //all data in first word
+            {
+               data[0] |= ival >> (31 - curPos->offsetBits);
+            }
+            else if ((curPos->offsetBits + curPos->numBits) >= 31) //all data in second word
+            {
+               data[1] |= ival >> (63 - curPos->offsetBits);
+            }
+            else //data spans across both words
+            {
+               data[0] |= ival << (curPos->offsetBits - 31);
+               data[1] |= ival >> (63 - curPos->offsetBits);
+            }
          }
-         else
+         else // little-endian
          {
-            data[0] |= ival << curPos->offsetBits;
+            if (curPos->offsetBits > 31)
+            {
+               // data entirely in the second word
+               data[1] |= ival << (curPos->offsetBits - 32);
+            }
+            else if ((curPos->offsetBits + curPos->numBits) <= 32)
+            {
+               // data entirely in the first word
+               data[0] |= ival << curPos->offsetBits;
+            }
+            else
+            {
+               // data spans both words
+               data[0] |= ival << curPos->offsetBits;
+               data[1] |= ival >> (32 - curPos->offsetBits);
+            }
          }
       }
 
@@ -453,8 +503,18 @@ void CanMap::ClearMap(CANIDMAP *canMap)
 int CanMap::Add(CANIDMAP *canMap, Param::PARAM_NUM param, uint32_t canId, uint8_t offsetBits, int8_t length, float gain, int8_t offset)
 {
    if (canId > MAX_COB_ID) return CAN_ERR_INVALID_ID;
-   if (offsetBits > 63) return CAN_ERR_INVALID_OFS;
-   if (length > 32 || length < -32) return CAN_ERR_INVALID_LEN;
+   if (length == 0 || ABS(length) > 32) return CAN_ERR_INVALID_LEN;
+   if (length > 0)
+   {
+      // little-endian mapping
+      if (offsetBits + length - 1 > 63) return CAN_ERR_INVALID_OFS;
+   }
+   else
+   {
+      // big-endian mapping
+      if (offsetBits > 63) return CAN_ERR_INVALID_OFS;
+      if (static_cast<int8_t>(offsetBits) + length + 1 < 0) return CAN_ERR_INVALID_OFS;
+   }
 
    CANIDMAP *existingMap = FindById(canMap, canId);
 

src/cansdo.cpp

@@ -330,7 +330,7 @@ void CanSdo::ReadOrDeleteCanMap(CAN_SDO* sdo)
          else if (sdo->subIndex & 1) //odd sub indexes have data id, position and length
             sdo->data = id | (canPos->offsetBits << 16) | (canPos->numBits << 24);
          else //even sub indexes except 0 have gain and offset
-            sdo->data = (((uint32_t)(canPos->gain * 1000)) & 0xFFFFFF) | (canPos->offset << 24);
+            sdo->data = (uint32_t)(((int32_t)(canPos->gain * 1000)) & 0xFFFFFF) | (canPos->offset << 24);
          sdo->cmd = SDO_READ_REPLY;
       }
       else
@@ -379,7 +379,11 @@ void CanSdo::AddCanMap(CAN_SDO* sdo, bool rx)
       else if (mapInfo.numBits != 0 && sdo->subIndex == 2) //This sort of verifies that we received subindex 1
       {
          //Now we receive gain and offset and add the map
-         mapInfo.gain = (sdo->data & 0xFFFFFF) / 1000.0f;
+
+         // sign extend the 24-bit integer to a 32-bit integer
+         int32_t gainFixedPoint = (sdo->data & 0xFFFFFF) << (32-24);
+         gainFixedPoint >>= (32-24);
+         mapInfo.gain = gainFixedPoint / 1000.0f;
          mapInfo.offset = sdo->data >> 24;
 
          if (rx) //RX map