While loop is true but stuck in it

chevremontagne · December 13, 2022, 8:54pm

Hi guys,

So here what I'm trying to do. I'm using one Arduino Uno (the master one) to read values of 2 MPU6050 via i2c. Then, I send a command through CAN bus using MCP2515 shields to a motor controller.

Then, I have a second Arduino Uno (slave) that also has a CAN bus shield and I use it to write data on an SD card. I use a push button to switch the state variable. Here's my problem:
I want to write data on the SD card coming from the motor controller and the data registered by the master Arduino. Requesting data to the motor controller is an easy task, because I send a CAN message with the RTR bit == 1 and I receive a answer with the desired data.

However, when I want to get data from the master, I don't know how to implement the RTR protocol. Hence, I simply always send the data from the sensors over the CAN bus and the slave will capt the data when it needs it.

The problem occurs in the readRegister() void. I'm trying to make sure I read the right message in the right order to make sure I have a consistent file at the end. This part is now no longer working and I have a problem specifically with this part :

(infoEnregistrer[i] == canId)

I simply want to confirm that the CAN id is the same as the one I want to write to my SD. When the code was working, I was getting the desired CAN id but I was still stuck in the While loop...
To be honest, it's been 2 days I'm on this problem and I have no clue why it's not working nor how to correct it.

#include "Wire.h"


//***********************************************************//
// For Arduino MCP2515 Hat:
// the cs pin of the version after v1.1 is default to D9
// v0.9b and v1.0 is default D10
const int SPI_CS_PIN = 10; // pin 9
const int CAN_INT_PIN = 2;
#include "mcp2515_can.h"
mcp2515_can CAN(SPI_CS_PIN); // set CS pin

#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x20, 16, 2);

byte axisId = 0x01 << 5U; // Result: 0x020
 
unsigned long periode = 60; // période d'aquisition de données
unsigned long delais; //Utilisé pour storer millis()
unsigned long timer;
unsigned long affichage = 0; // pour diminuer le refresh rate du LCD
unsigned long affichage2 = 0; // pour diminuer le refresh rate du LCD
unsigned long affichage3 = 0; // pour diminuer le refresh rate du LCD

/**********Play/pause datalogger**************/
const byte interruptPin = 3;
volatile byte state = LOW;

boolean ledState = LOW;  // ledState used to set the LED
const int ledLight = 7;  // the number of the LED pin
const int ledRead = 6; // pin pour play/pause enregistrement
unsigned long attendre = 0; // delais pour empêcher 8 fois la meme lecture du bouton pour enregistrement
byte infoEnregistrer[] = {0x037, 0x029, 0x035};//, 0x049}; // voltage, vitesse, température, angle

byte lastId;
#include <SdFat.h>
SdFat sd;
SdFile myFile;
char filename[] = "test000.txt";
int i = 0;
int j = 0;
bool flag = true;

/*************************Debut du programme*******************************************/

void setup() {
  Wire.begin();
  lcd.init();
  lcd.backlight();
  lcd.setCursor(0,0);
  while (!Serial) ; // wait for Arduino Serial Monitor to open
  Serial.begin(115200);
    while (CAN_OK != CAN.begin(CAN_500KBPS)) {             // init can bus : baudrate = 500k and clock 8MHz for Odrive Pro MCP_8MHz
      SERIAL_PORT_MONITOR.println("CAN init fail, retry...");
      delay(100);
  }
  SERIAL_PORT_MONITOR.println("CAN init OK.");
  lcd.setCursor(0,0);
  lcd.print("CAN init OK.");
  
  if (!sd.begin(4, SPI_FULL_SPEED)) sd.initErrorHalt();
  while (sd.exists(filename)){
    i ++;
    sprintf(filename, "test%03i.txt", i); 
  }
  Serial.print(filename);
  if (!myFile.open(filename, FILE_WRITE)) { ;//O_RDWR | 
    sd.errorHalt("opening test.txt for write failed");
  }
  SERIAL_PORT_MONITOR.println("SD init OK.");
  
  lcd.setCursor(0,1);
  lcd.print("SD:");
  lcd.setCursor(0,9);
  lcd.print(filename);
  
  pinMode(ledLight, OUTPUT);
  pinMode(interruptPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin), blink, FALLING);
  delay(5000);
  delais=millis();
  timer = millis();
}

void loop() {
  if ((millis()-delais)>periode){
    voltage();
    vitesse();
    temperature();
    //getAngleCAN();
    digitalWrite(ledLight, state);
    delais = millis();
  }
}
void blink() {
  state = !state;
}
void readRegister(char *info, bool deux, byte filter){ // si le get message retourne 2 données différentes/ filter : savoir quel message est renvoyer
  if (state == HIGH){
    lcd.setCursor(13,1);
    lcd.print("REC");
    
    unsigned char len = 0;
    unsigned char buf[8];
    float y,z;
    
    if (CAN_MSGAVAIL == CAN.checkReceive()) {         // check if data coming
      CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
      byte canId = CAN.getCanId(); //unsigned long 
      for (i = 0; sizeof(infoEnregistrer)-1; i++){
        j=0;
        flag = true;
        while (flag == true) {
          CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          byte canId = CAN.getCanId(); //unsigned long 
          if (infoEnregistrer[i] != canId){
            Serial.print("obtenu :");
            Serial.print(canId);
            Serial.print(", demandé:");
            Serial.print(infoEnregistrer[i]);
            Serial.print(", ");
            Serial.println(infoEnregistrer[i] == canId);
          }
          else{
            Serial.print("obtenu :");
            Serial.print(canId);
            Serial.print(", demandé:");
            Serial.print(infoEnregistrer[i]);
            Serial.print(", ");
            Serial.println(infoEnregistrer[i] == canId);
            flag == false;
            break;
          }
        }
          SERIAL_PORT_MONITOR.println(canId);
          Serial.print(canId);
          Serial.println(" Victoire");
          union u_tag {
              byte b[4];
              float fval;
          } u;
          u.b[0] = buf[0];
          u.b[1] = buf[1];
          u.b[2] = buf[2];
          u.b[3] = buf[3];
          y = u.fval;
          
          if (deux ==1){
            u.b[0] = buf[4];
            u.b[1] = buf[5];
            u.b[2] = buf[6];
            u.b[3] = buf[7];
            z=u.fval;
          }
          
          if (canId == 0x037 && (millis()-affichage)>500){ //voltage
            lcd.setCursor(0,0);
            lcd.print(String(y) + " V ");
            affichage = millis();
          }
          else if (canId == 0x035 && (millis()-affichage3)>500){ //temp
            lcd.setCursor(0,1);
            lcd.print(String(y) + " C    ");
            affichage3 = millis();
          }
          else if (canId == 0x049 && (millis()-affichage3)>500){ //temp
            lcd.setCursor(9,0);
            lcd.print(String(y-z) + " deg  ");
            affichage3 = millis();
          }
            writeSD(y,z);
       }
      }
    }
 else if (state == LOW){
  lcd.setCursor(0,0);
  lcd.print("Blanc: reset    ");
  lcd.setCursor(0,1);
  lcd.print("Bleu: REC       ");
  }
}
void getAngleCAN(){
  readRegister("lecture acceleros", 1, 0x049);
  myFile.println();
  myFile.sync();
}
void voltage(){
  byte getVBus = 0x017; // Get_Vbus_Voltage_Current
  byte getVBusId = axisId | getVBus; // Result:  0x037
  CAN.sendMsgBuf(getVBusId, 0, 1, 8, 0, true);
  readRegister("voltage & ampère", 1, getVBusId);
}
void vitesse(){
  byte getVel = 0x009; // Get_Encoder_Estimates 
  byte getVelId = axisId | getVel; // Result:  0x029
  CAN.sendMsgBuf(getVelId, 0, 1, 8, 0, true);
  readRegister("position & vitesse", 1, getVelId);
}
void temperature(){
  byte getTemp = 0x015; // Get_Temperature
  byte getTempId = axisId | getTemp; // Result:  0x035
  CAN.sendMsgBuf(getTempId, 0, 1, 8, 0, true);
  readRegister("temperature", 0, getTempId);
}
void writeSD(float y, float z) {
  myFile.print(y);
  myFile.print(",");
  myFile.print(z);  
  myFile.print(",");
}

Thanks in advance !

chevremontagne · December 13, 2022, 9:08pm

Here's a simplified version of the problematic section.

void readRegister(char *info, bool deux, byte filter){ // si le get message retourne 2 données différentes/ filter : savoir quel message est renvoyer
  if (state == HIGH){
    unsigned char len = 0;
    unsigned char buf[8];
    float y,z;
    
    for(i = 0; sizeof(infoEnregistrer)-1; i++){
      if (CAN_MSGAVAIL == CAN.checkReceive()) {
        CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
        byte canId = CAN.getCanId(); //unsigned long 
        while (infoEnregistrer[i] != canId){
          CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          byte canId = CAN.getCanId(); //unsigned long         
        }
          union u_tag {
            byte b[4];
            float fval;
          } u;
          u.b[0] = buf[0];
          u.b[1] = buf[1];
          u.b[2] = buf[2];
          u.b[3] = buf[3];
          y = u.fval;
          if (deux ==1){
            u.b[0] = buf[4];
            u.b[1] = buf[5];
            u.b[2] = buf[6];
            u.b[3] = buf[7];
            z=u.fval;
          }
          writeSD(y,z);
        
      }
    }
  }
}

MicroBahner · December 13, 2022, 9:50pm

chevremontagne:

        byte canId = CAN.getCanId(); //unsigned long       // <--- first variable and it is byte, NOT long
        while (infoEnregistrer[i] != canId){               //  <-- the while uses your first defined variable-
          CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          byte canId = CAN.getCanId(); //unsigned long  // <---here you define a different variable, that is never used!

Be aware, that you define two different variables with the same name! Learn about the scope of a variable. Your second canId is newly created with every while loop, but never used. Omit the 'byte' when using canId within the while loop.
And if you really need an unsigned long ( as in your comment ), than you should not use 'byte' as the variable type, but really 'unsigned long'.

ToddL1962 · December 13, 2022, 9:50pm

The canId you have declared in the while code block is not the one being tested in the while loop conditional.

        byte canId = CAN.getCanId(); 
        while (infoEnregistrer[i] != canId){
          CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          byte canId = CAN.getCanId();
        }

[/quote]

chevremontagne · December 13, 2022, 10:30pm

Effectively, I'm only using canId in my While loop, because I want to make sure I have the right message. Even if it is newly created every iteration, it is not really a problem (I will still change it and declare it only at the top)
Sincerely, I don't really see the difference between unsigned long, unsigned char and byte, when I see examples on Internet, I take the same variable declaration.

chevremontagne · December 13, 2022, 10:38pm

If I declare canId at the top and use it as a variable at both places, then it is going to be tested in the while loop ?!
Just to be sure, because I made the modification, but it still isn't working..
Maybe I have another problem

Grumpy_Mike · December 13, 2022, 11:00pm

So post your new code and we can tell what you did wrong.

That is fundamental to learning how to program. Look up variable types in the reference section, found under the help menu in the IDE.

chevremontagne · December 13, 2022, 11:11pm

Ok so here's the new void, I decided to simplify it and include the variable inside it instead of passing it to make sure there's no error coming from that.

void readRegister(){ 
  if (state == HIGH){
    unsigned char len = 0;
    unsigned char buf[8];
    float y,z;
    
    for(i = 0; (sizeof(infoEnregistrer))>i; i++){
      Serial.println(i);
      CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
      CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
      canId = CAN.getCanId(); //unsigned long 
        while (infoEnregistrer[i] != canId){
          CAN.sendMsgBuf(infoEnregistrer[i], 0, 1, 8, 0, false);
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          canId = CAN.getCanId(); //unsigned long    
          Serial.print("Id requested: ");
          Serial.print(infoEnregistrer[i]);
          Serial.print(", id received: ");
          Serial.println(canId);
        }
          union u_tag {
            byte b[4];
            float fval;
          } u;
          u.b[0] = buf[0];
          u.b[1] = buf[1];
          u.b[2] = buf[2];
          u.b[3] = buf[3];
          y = u.fval;

            
          u.b[0] = buf[4];
          u.b[1] = buf[5];
          u.b[2] = buf[6];
          u.b[3] = buf[7];
          z=u.fval;
          
          writeSD(y,z);
          Serial.println("SD");
      }
    
  }
}

chevremontagne · December 14, 2022, 2:43am

With that changes being done, the only problem resides in the transform of the values. When I print them, I don't get the values I'm supposed to.
For example, for the infoEnregistrer[0], it is supposed to be approximatly 57.4, however I get a value of -20.0 or something like that. Is it because of my union u_tag or because of the type of variable assigned ?!

gfvalvo · December 14, 2022, 3:33am

That likely won't happen since 'byte' is an integer type:

Grumpy_Mike · December 14, 2022, 7:11am

Please, please, please use the correct word. Void literally means "nothing".

So you just said:-

so here's the new nothing.

What you posted is called a function.

chevremontagne · December 14, 2022, 12:40pm

Well it is not a function as it doesn't return any value. I read it on another post.

Since it's not the core of my question is it possible to answer the original question.

UKHeliBob · December 14, 2022, 12:53pm

How wrong can you be ?

b707 · December 14, 2022, 12:57pm

You need to learn it if you want to fix your problem:

For example...

I think you got exactly 20, because type of infoEnregistrer[0] is a byte, so it can contains only integers between 0 and 255. So it never give you 57.4 or any other float value.

i think you should put this project aside and read a programming textbook.

alto777 · December 14, 2022, 1:06pm

I grew up before the internet, well at least let's say I was older when it came up, and was taught to not believe everything I read.

That is only several orders of magnitude more important now.

Don't believe everything you read. There are perhaps subtle differences in terminology that vary from language to language.

In C/C++ the word function means something.

a7

Grumpy_Mike · December 14, 2022, 1:10pm

You misunderstood what that post says.

But it is the core to solving your problem. You do not know the right words to use, or even seem to recognise what words mean. It is vital that you learn that.

Our aim is to educate you and knowing what words mean is a big part of what we are trying to do here. You seem a bit disinterested in learning, and just want someone to correct your code. But until you can understand technical words and there meaning, then no one can tell you what is wrong.

Only you have this particular hardware so only you can fix it and make sure it behaves like you want it to.

So advice:-

Do not use single letter variable names, use descriptive names that give a hint into what they do.
Use the serial print command to see what you are getting back from your CAN bus. Then every step of the way you can see what you are getting and spot where it is going wrong.
Stop ignoring advice and make your variables of a suitable type for the data they are going to hold.

b707 · December 14, 2022, 1:14pm

the most interesting thing is that the message quoted by the @chevremontagne does not say anywhere that the void function is not a function.
On the contrary, it says twice that the void function is a function too like any other function

StefanL38 · December 14, 2022, 1:29pm

Can you see my cup of coffee on my desk?
No! I haven't installed the webcam and you don't receive the picture of my webcam.

Still my cup of coffee is existing!

There is a

BIG

difference between

unsigned long,
unsigned char
and byte,

Though you don't know the difference yet. Because you haven't read enough details about their differencies!

As soon as you have read these details and these details belong to your knowledge you will understand it.

How can you judge from a too small knowledgebase? You seem to have decided
"I know enough about variable types".

Others users here give you a heads up "you should learn some more about variable types"
because they can clearly see the lack of knowledge on your side.

To me this attitude has an aspect of beeing somehow to some level ignorant.

If you want to finish your project faster you should learn more basics.
Knowing more basics will accelerate writing code more than the learning the basics has distracted finishing your project.

best regards Stefan

chevremontagne · December 14, 2022, 1:34pm

Exept the firsts answers I got, your answers aren't really helpful when it's just a comment. I understand that you might think I sould learn more about variable types, but there's always a manner to exprimate it. Also, to give reliable ressources to the person. When it comes to just commenting it, you better keep it to yourself. As there is a manner to write a question, there should be a manner to answer it.

UKHeliBob · December 14, 2022, 1:37pm

That would be a good start

Take a look at the Data Types section of Arduino Reference - Arduino Reference

Topic		Replies	Views
CAN BUS Communication Networking, Protocols, and Devices	6	502	August 27, 2024
While loop not advancing Programming	4	414	May 6, 2021
Help with buffers and CAN bus Programming	3	4834	May 5, 2021
HELP with CAN BUS Receive Data Programming	9	2161	February 28, 2023
CAN message using 2 Arduino Nano Programming	5	414	May 6, 2021

While loop is true but stuck in it

BIG

Related topics