Mileage from can-bus

Getting mileage from can-bus works ok.

if (canId == 0x540) {
      uint32_t odometer = (uint32_t)rxBuf[7] <<16;
      odometer |=  rxBuf[6]<<8;
      odometer |=  rxBuf[5];

     Serial.println(odometer);  //for test

Then need to make own can-frame to dash app, but it shows 0 there. I confirmed that all in XML which is needed for this is done right.
I want to print that what it is sending to an application to a serial monitor for testing and exclude possible errors in XML.

This to serial monitor:

void SendCANFramesToSerial()
{
  byte buf[8];

  buf[0] = (0xff);
  buf[1] = (0xff);
  buf[2] = (0xff);
  buf[3] = (0xff);
  buf[4] = (0xff);
  buf[5] = ((odometer  & 0xff);
  buf[6] = ((odometer >> 8) & 0xff);
  buf[7] = ((odometer >> 16) & 0xff);

These does not print correct value:

Serial.println(buf[5] + buf[6] + buf[7])
Serial.println(buf[5]  & 0ff) + (buf[6] << 8) +  (buf[7] << 16);

well obviously the first solution doesn't

Serial.println(buf[5] + buf[6] + buf[7])

and neither does the 2nd (simply because you shiftout of the variable's size.
try

Serial.println(   (uint32_t) buf[6] << 8 |  (uint32_t) buf[7] << 16 | buf[5] ); 

and use | (bitwise or) instead of '+'
also the bitmask is obsolete since you are casting into an 8 bit variable. (byte)

byte buf[8];

  buf[0] = (0xff);
  buf[1] = (0xff);
  buf[2] = (0xff);
  buf[3] = (0xff);
  buf[4] = (0xff);
  buf[5] = ((odometer  & 0xff);
  buf[6] = ((odometer >> 8) & 0xff);
  buf[7] = ((odometer >> 16) & 0xff);
if (canId == 0x280) {
      uint32_t rpm = (uint32_t)rxBuf[2] <<8;
      rpm |=  rxBuf[3];     

  buf[0] = (0xff);
  buf[1] = (0xff);
  buf[2] = ((rpm >> 8) & 0ff);
  buf[3] = (rpm & 0ff);
  buf[4] = (0xff);
  buf[5] = ((odometer  & 0xff);
  buf[6] = ((odometer >> 8) & 0xff);
  buf[7] = ((odometer >> 16) &  0xff);

I have rpm like this in sketch and it works ok in app.

that looks like a typo, but as said, you are bit masking an 8 bit variable, with a bitmask where al 8 bits are set, that is redundant.

uint32_t rpm = (uint32_t)rxBuf[2] <<8;
      rpm |=  rxBuf[3];   

yes that works as well. Main thing to keep in mind with bitshifting is the variable size

Yes, sorry for typos.
I just do not understand why this rpm works but odometer not.

you missed a cast, and are shifting all 8 bits out of the variable (which is 8 bit),
odometer |= (uint32_t) rxBuf[6]<<8;

I think i understand bitshift wrong.
In this case byte 5 is 00011101, byte 6 = 01111000, byte 7 = 00000111.

What it exactly do in this?

if (canId == 0x540) {
      uint32_t odometer = (uint32_t)rxBuf[7] <<16;
      odometer |=  rxBuf[6]<<8;
      odometer |=  rxBuf[5];

What happen to byte7 for example? Why it need to shift to left 16?
In my understanding it is just eight zeros then.

out of

byte 7 = 00000111, byte 6 = 01111000 & byte 5 = 00011101

you want to create a single value.

 00000111 01111000 00011101

for byte 7 this means that first you should cast it to a variable that is large enough to hold the new value.

00000111  in a 32-bit, will be 00000000 00000000 00000000 00000111

then you bitshift this to the left 16 bits.

00000000 00000111 00000000 00000000

you do the same for byte 6, but only 8 bits to the left.

00000000 00000000 01111000 00000000

and for byte 5, since there is no shifting taking place, there is no need to increase the size of the variable, but if you would it would look like this

00000000 00000000 00000000 00011101

now you perform a bitwise 'or'

00000000 00000111 00000000 00000000
00000000 00000000 01111000 00000000
00000000 00000000 00000000 00011101
-----------------------------------------------------------
00000000 00000111 01111000 00011101

I do not know a more basic way to explain it than this.

If confronted with this situation programatically it can also be done in a loop where you shift the output value (rather than the input)

uint8_t i = 3;
uint32_t output = 0;
while (i) {
  i--;
  output = output << 8; // the first time through the iteration you shift '0' and the result is none
  output |= input [i];  // like this input[2] will be msB and input[0] lsB
}

this is just another way of doing the same thing.

So if no bitshift

00000111 
01111000 
00011101
-----------------
01111111

Result is 127 and it is incorrect, so that is why it need bitshift, right?

yes to move the bits to the correct spot in the output variable.

keep in mind that if you shift

00000000 = 00000111 << 8 

that the variable is big enough to hold the result

Thank you for explanation.

I still do not understand when making that new can-frame for an application, does it "disassemble" again that single value?
Rpm goes to gauge correctly, but odometer shows 0.

 buf[0] = (0xff);
  buf[1] = (0xff);
  buf[2] = ((rpm >> 8) & 0xff);
  buf[3] = (rpm & 0xff);
  buf[4] = (0xff);
  buf[5] = ((odometer  & 0xff);
  buf[6] = ((odometer >> 8) & 0xff);
  buf[7] = ((odometer >> 16) &  0xff);

yes.

I have to admit that i find it strange that the RPM sends MSB first and the odometer sends LSB first. That is just such an unlikely format. Which is the correct way.
(btw for the rest there is nothing wrong, but maybe somewhere else something is not correct)

Okay, this how i have that rpm now, give wrong value first, but when it goes to application in new frame it shows there right rpm in gauge.

if (rxId == 0x280) {
      uint16_t rawRPM = (uint16_t)rxBuf[2] << 8;
      rawRPM |= rxBuf[3];
      rpm = rawRPM/4;

In serial monitor it give wrong values, so, that new frame change it to right.

If i change places of rxBuf2 and 3 then it shows right rpm in serial monitor, but wrong in app.

so what are you doing in the Serial output. Clearly you should be doing te same in both.

Now rxBuf 2 and 3 are swapped, also buf 2 and 3 of course, RPM works fine, odometer stays 0, even i tried to swap in all ways.

if (rxId == 0x280) {
      uint16_t rawRPM = (uint16_t)rxBuf[3] << 8;
      rawRPM |= rxBuf[2];
      rpm = rawRPM/4;
}

 else if (rxId == 0x520) {
      uint32_t odometer = (uint32_t)rxBuf[7] << 16;
      odometer |= rxBuf[6] << 8;
      odometer |= rxBuf[5];
}

 buf[0] = (0x49);
  buf[1] = (0xff);
  buf[2] = (rpm & 0xff);  //was ((rpm >> 8) & 0xff);
  buf[3] = ((rpm >> 8) & 0xff);  //was (rpm & 0xff);
  buf[4] = (0xff);
  buf[5] = (odometer & 0xff);                   // tried to swap these also, does not work
  buf[6] = ((odometer >> 8) & 0xff);
  buf[7] = ((odometer >> 16) & 0xff);

Putting odometers together as before for serial monitor, it give correct value.

i hate having to repeat myself, see reply #6

Thank you and sorry.

With this it did not work at first.

else if (rxId == 0x520) {
      uint32_t odometer = (uint32_t)rxBuf[7] << 16;
      odometer |= (uint32_t) rxBuf[6] <<8;
      odometer |= rxBuf[5];

But this way it finally works.

else if (rxId == 0x520) {
      uint32_t rawOdo = (uint32_t)rxBuf[7] << 16;
      rawOdo |= (uint32_t) rxBuf[6] <<8;
      rawOdo |= rxBuf[5];
      odometer = rawOdo;

"char" values used in expressions are upgraded to "int" so it is possible to shift a "char" left by 8 bits and not lose data.

Lol. You aren't the only one who gets turned around using them. :wink:

This was a 'byte' and an array, the bits get lost.

Oh did you have another variable with the same name (but a different scope).
In that case, actually

else if (rxId == 0x520) {
      odometer = (uint32_t)rxBuf[7] << 16;
      odometer |= (uint32_t) rxBuf[6] <<8;
      odometer |= rxBuf[5];

that may also work.