Longs printing as signed integers

Projects Programming
1

I'm trying to read and print wheel encoder values on a Mega 2650, and am having
great difficulty. I've tried several variations of

void EncoderLt() {
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  if(Serial3.available() > 3) {
    a1 = Serial3.read();
    a2 = Serial3.read();
    a3 = Serial3.read();
    a4 = Serial3.read();
   long result1;
    result1 = a1<<24;
    result1 += a2<<16;
    result1 += a3<<8;
    result1 += a4;
    Serial.println(result1);
  }
}

The serial Tx is to a Devantech MD49 motor controller; the a's are typed bytes. The count proceeds normally until the value reaches 2^15, then the count reverts to negative values and counts down toward zero. In other words, the output behaves like a signed integer instead of a long. I've printed out the a's and switch occurs when a3 = 127. I've also tried

  1. typing the Serial.reads as long
  2. typing and shifting the reads in a single statement into a long.

All give the same result.

Any insight is welcomed,
John-

2

I believe a "long" is a long signed int by default. You need to specify unsigned, if that's what you want.
source:
https://en.cppreference.com/w/cpp/language/types

1 Like
3

What are the data types for a1, a2 ...? I suggest uint8_t.

4

The long needs to be signed, some values can be negative, depending if the encoder is measuring forward or reverse motion.
Surely a long has more digits than an signed int (32 bits vs 16 bits).

John-

5

Was bytes, however, one version of the code used:

void EncoderLeft(){
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  if(Serial3.available() > 3) {
    a1 = Serial3.read();
    a2 = Serial3.read();
    a3 = Serial3.read();
    a4 = Serial3.read();
    result1 = a1<<24ul;
    result1 += a2<<16ul;
    result1 += a3<<8ul;
    result1 += a4;
    Serial.println(result1);
  }
}
  Does that qualify?

Another version used:


void EncoderLeft(){
long encLt;  
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  if(Serial3.available() > 3) {
  encLt = long(Serial2.read())<<24;      
  encLt += long(Serial2.read())<<16;
  encLt += long(Serial2.read())<<8;
  encLt += long(Serial2.read());
  Serial.println(encLt);
  }
}

That printed large, negative, constant values.

I've tried a lot of perturbations.

Thanks,
John-

6

try:

void EncoderLt() {
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  unsigned long  result1 = 0;
  if (Serial3.available() > 3) {
    for (uint8_t i = 0; i < 4; i++) {
      result1 = result1 << 8;
      result1 |= Serial3.read();
    }
    Serial.println(result1);
  }
}
7

Please post all the code. Snippets are frowned upon, as they leave out very important information.

At the moment, this statement makes no sense at all:

switch occurs when a3 = 127.

8

do you mean "when a1 > 127" ?

9

No, that's the mystery.

More guessing and it seems to be working.

void EncoderLeft(){
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  if(Serial3.available() > 3) {
    a1 = Serial3.read();
    a2 = Serial3.read();
    a3 = Serial3.read();
    a4 = Serial3.read();
    long result1;
    result1 = long(a1)<<24;
    result1 += long(a2)<<16;
    result1 += long(a3)<<8;
    result1 += long(a4);
    Serial.println(result1);
  }
}

I still have no idea what's going on but for now I'll top irritating everyone.

Thanks to all,
John-

10

Since you can't be bothered to post all the code, neither do we. But no matter.

2 Likes
11

My apologies, here's the full code.

/*********************************************************
                         ARDUINO Mega
               SAMS (from Recycle-II-MileOneV30)


**********************************************************/
#include <stdint.h>
#include <Wire.h>
#include <EEPROM.h>

#define  SynchByte 0x00          // MD49 Command byte

//    ****  RC  ****
byte rc_active   = 33;       //Signal from NanoEvery indicating R/C Control when high
byte busy;
byte a1=0;
byte a2=0;
byte a3=0;
byte a4=0;
byte b1=0;
byte b2=0;
byte b3=0;
byte b4=0;

//    ****  Motors  ****
byte SetMode = 0x34;
byte MtrMed = 238;

//    ****  Encoders  ****
long encoderLt;
long encoderRt;

// ------------------------ START CODE --------------------------------------
void setup()  {
  delay(500);
  InitPinModes();  
  InitSerialPorts();
  ConfigMotors();    
  EncRst();
  delay(50);   
}

void loop()  {
    busy = digitalRead(rc_active);          //busy = 0 when in Auto Mod
    if(busy == 0) {
      MtrSpeed(MtrMed, MtrMed);
      Serial.print(millis());
      Serial.write(9);
      Serial.print(a1);
      Serial.write(9);
      Serial.print(a2);
      Serial.write(9);
      Serial.print(a3);
      Serial.write(9);
      Serial.println(a4);
      EncoderLeft();
      Serial.print("     ");
      EncoderRight();      
      Serial.println("");
    
      delay(250);
    }      
 
    else {   //  busy != 0 when motor control is switched to STOP or DRIVE mode 
      Serial1.println("  idle  ");
      EncRst();
      delay(20);
    }
}
//-----------------------------------------------------------
 
void InitPinModes () {
//  pinModes for sonars set in Ranging Module 
  pinMode(rc_active, INPUT);            // zero in Auto mode
}
//--------------------------------------------

void InitSerialPorts(){
  Serial.begin(9600);                    //Monitor
  Serial1.begin(19200);                  //Logomatic communications
  Serial3.begin(38400);                  //MD49 Motor Controller
}
//-------------------------------------------

void ConfigMotors(){
  //  Begin by Setting & Stopping Motors
  Serial3.write(SynchByte);                  //Set Motor Controller Mode to independent motor control
  Serial3.write(SetMode);
  Serial3.write(0x00);
  delay(20);
  Serial3.write(SynchByte);                  //Disable Timeout - Turn motor controller on, then reset Arduino
  Serial3.write(0x38);
  delay(20);
  Serial3.write(SynchByte);                  //Turn Off(0x36);  On (0x37) Speed Control (encoder feedback)
  Serial3.write(0x36);
  delay(20);
//  Begin by Stopping Motors   
  Serial3.write(SynchByte);                  //Left Motor Stop
  Serial3.write(0x31);
  Serial3.write(128);
  Serial3.write(SynchByte);                  //Right Motor Stop
  Serial3.write(0x32);
  Serial3.write(128);
  delay(20);
  Serial3.write(SynchByte);                  //Set Acceleration time constant
  Serial3.write(0x33);
  Serial3.write(0);
  delay(20);
  Serial3.write(SynchByte);                  //Reset Encoders
  Serial3.write(0x35);  
}
//---------------------------------------

void MtrSpeed(byte LtSpeed, byte RtSpeed) {
      Serial3.write(SynchByte);      //Left Motor      
      Serial3.write(0x31);
      Serial3.write(LtSpeed); 
      delay(20);     
      Serial3.write(SynchByte);      //Right Motor
      Serial3.write(0x32);
      Serial3.write(RtSpeed);
      delay(20);
}
//-------------------------------------------

void EncoderLeft(){
  Serial3.write(0x00);
  Serial3.write(0x23);
  delay(50);
  if(Serial3.available() > 3) {
    a1 = Serial3.read();
    a2 = Serial3.read();
    a3 = Serial3.read();
    a4 = Serial3.read();
    long result1;
    result1 = long(a1)<<24;
    result1 += long(a2)<<16;
    result1 += long(a3)<<8;
    result1 += long(a4);
    Serial.println(result1);
  }
}

void EncoderRight() {
  Serial3.write(0x00);
  Serial3.write(0x24);
  delay(50);
  if(Serial3.available() > 3) {
    b1 = Serial3.read();
    b2 = Serial3.read();
    b3 = Serial3.read();
    b4 = Serial3.read();
    long result2;
    result2 = long(b1)<<24;
    result2 += long(b2)<<16;
    result2 += long(b3)<<8;
    result2 += long(b4);
    Serial.println(result2);
  }
}
//--------------------------------------

void EncRst() {
  Serial3.write(SynchByte);                  //Reset Encoders
  Serial3.write(0x35);
  delay(50);
}

This version finally works. The only difference with the previous versions is in the previously code bits for void EncoderLeft() and void EncoderRight()

John-

12

Regarding data types, i highly recommend specifying them directly. You know what your using then.

An 8 bit atmega chip, int is 16 bit
An arm microntroller (r4, esp32, teensy) an int is 32 bit.

Best of using
int8_t
uint8_t
int16_t
uint16_t
int32_t
uint32_t

1 Like
13

I think that in order for you to assemble a long from individual bytes, you have to do the assembly with mostly "unsigned" components.

In the above code, your sign bit is only in the first byte read, but if the subsequent bytes are > 127, the cast may result in negative numbers being added to encLt.
Maybe. Serial2.read() should be returning a (signed) int that should never be negative. OTTH, that means that x<<24 will result in a "signed integer overflow", which has "undefined behavior."

2 Likes
14

You must use unsigned long in bits shifting rather than long

1 Like
15

The byte is promoted to int, which is fine; but when shifted 16 or more, all the bits are shifted "off the (left) end", leaving zero. So the first value that has some effect is a3

Making the bit shift count long has no effect. At most you'll be shifting 64 places.

Typo: available is called on Serial3, but the read is on Serial2. So those will probably all return -1, and then get cast to long before shifting

  FF 00 00 00
  FF FF 00 00
  FF FF FF 00
+ FF FF FF 00
-------------
  FE FE FE FF ==> -16843009

Because of addition, the bits carry. Don't use addition when doing this kind of bit shuffling and assembly, use bit-OR |=

The long is negative only if the first (unsigned) byte is >= 128. The sign/magnitude of the other three bytes does not matter. Does that make sense?

You should really only do left-shift on unsigned values. With a sign, the result can "flip-flop" depending on what ends up on the high bit, and is confusing.

  Serial.print("0xA0 << 7 = ");
  Serial.println(0xA0 << 7);    // 20480
  Serial.print("0xA0 << 8 = ");
  Serial.println(0xA0 << 8);    // -24676
  Serial.print("0xA0L << 8 = ");
  Serial.println(0xA0L << 8);   // 40960
  Serial.print("0xA0 << 9 = ");
  Serial.println(0xA0 << 9);    // 16384
  Serial.print("0xA0 << 10 = ");
  Serial.println(0xA0 << 10);   // -32768

You can avoid the casts to long and manual multiples of eight with something like

void mush_unroll(byte a1, byte a2, byte a3, byte a4) {
  long result = a1;  // probably should be uint32_t: explicit size and unsigned
  result <<= 8;
  result |= a2;
  result <<= 8;
  result |= a3;
  result <<= 8;
  result |= a4;
  Serial.print(result, HEX);
  Serial.print(" unroll ");
  Serial.println(result, DEC);
}
16

Thanks, I've never seen anything like this before. Works going forward and, as expected, when the encoders go in the opposite direction, the values decrement from FFFFFFFF.

John-

17

Sorry for the Serial2 typo; editing error. The executed code used Serial3.

John-

18

The initial code was very long and included many functions not relevant (my judgement) to the problem, so I just posted the code I suspected was causing the error. Upon your comment, I extracted the encoder function, then wrote the minimum code to employ it. I'm sorry to have irritated you.

John-

19

New posters often claim that, and they are very often wrong. The strongly recommended alternative is to post the minimal code that will compile and demonstrate the problem.

20

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.