CAN.h _id bug

31 Jan 2013

There is a bug when sending 29bit ID's. The CANMessage() function defines _id as an (int), it should be (long).

// this...
CANMessage(int _id, const char *_data, char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) {
      len    = _len & 0xF;
      type   = _type;
      format = _format;
      id     = _id;
      memcpy(data, _data, _len);
    }

//should be like this
CANMessage(long _id, const char *_data, char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) {
      len    = _len & 0xF;
      type   = _type;
      format = _format;
      id     = _id;
      memcpy(data, _data, _len);
    }

It also needs to be changed int can_helper.h file as well.

31 Jan 2013

No, there's no bug. On mbed an int is 32 bit wide (it already corresponds to long integer in other 8 bit micros). 16 bit int on mbed would be specified as short. So the 29 bit id fits easily into a 'normal' int on mbed.

Best regards
Neni

31 Jan 2013

Nenad Milosevic wrote:

No, there's no bug. On mbed an int is 32 bit wide (it already corresponds to long integer in other 8 bit micros). 16 bit int on mbed would be specified as short. So the 29 bit id fits easily into a 'normal' int on mbed.

Best regards
Neni

Bad practice to use type int in an application where you assume it's 32 or 16 bits.

You'll see a lot of C code with uint32_t alpha; or int16_t beta; or uint8_t delta;

to eliminate ambiguity. I don't like using "int" or "char".

Those typedefs are normally in stdint.h as I recall.

31 Jan 2013

Tbh there is really no reason to assume an int wouldn't be 32-bit on a 32-bit platform. And char is afaik always 8 bits. Only problem with char is that it isnt always clear if it is signed or unsigned. Anyway see here the data types used by the mbed compiler: http://mbed.org/handbook/C-Data-Types

31 Jan 2013

what's a 32 bit platform? Many CPUs/platforms have dynamically changeable modes, e.g., ARM7 TDMI can be 32 bit ARM and in a microsecond it switches to THUMB mode and is a 16 bit processor. Best not to assume anything about size of int, scalars. That's why stdint.h came to be, and sizeof(type)

But foremost, the reason not to use int is to make the code portable.

31 Jan 2013

Nenad Milosevic wrote:

No, there's no bug. On mbed an int is 32 bit wide (it already corresponds to long integer in other 8 bit micros). 16 bit int on mbed would be specified as short. So the 29 bit id fits easily into a 'normal' int on mbed.

Best regards
Neni

Ah ok, yeah i've been working with 16 bit micros for the last 7 or so years and just now coming over to 32 bits. But yeah if uint32_t was used then i wouldn't have assumed (int) was 16 bits.

03 Feb 2013

Good discussion above - for my 2¢, I would favor explicit definition of it being 32-bits wide, simply because I work on a variety of projects ranging from 8 to 32 bit and this keeps it more clear in my head.

There is one element of this that seems to have gone unnoticed. That would be the use of a signed vs. unsigned type. A CAN identifier, at least for most purposes, is probably not associated with a signed type, and any manipulation of the identifier is also most likely not based on a signed type. So, I would suggest that it be more appropriately defined as uint32_t, even though a 29-bit identifier does not set the highest bit so cannot be interpreted as negative.