Command Processor Object lifetime

Status
Not open for further replies.

DanielO

Member
I am using gcode to command a machine. A Teensy 4.0 is the controller recieving the data via serial and places them in a queue. When nothing is processing i pop a command from the queue.

I have three variables: previousc, currentc and nextc. At the beginning of each loop I move currentc to previousc nextc to currentc and then I pop a command from the queue it gets stored in the nextc variable.

My controller crashes when acessing the nextc variable (when doing a NULL check in case the buffer is empty). I have boiled it down to the code below but i dont understand why it crashes. My guess is that it is about object lifetime but i dont know... What is wrong?

Code:
class moveXY
{ 
  public:
  int x; 
  int y;
};

moveXY * nextMove; 
moveXY _nextMove = *new moveXY();
   
void setup() {
  _nextMove.x = 88;
  nextMove = &_nextMove;
}

void loop() {
    _nextMove = * new moveXY();
    _nextMove.x = 883;
    nextMove = &_nextMove;
    Serial.print("\n _nextMove = ");Serial.print(_nextMove.x);
    if(nextMove)
      Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
    delay(1000);
  
    nextMove = NULL;

    if(nextMove)  
      Serial.print("\n nextMove = ");Serial.print((*nextMove).x);

}
 
This :
moveXY * nextMove;

Shows that nextMove is a pointer. Setting that pointer to NULL with :
nextMove = NULL;

Means any write to that null ptr will fault.

Not seeing where that is in above code as it seems to only read when it has a valid pointer - but setting nextMove = NULL; could do it.

Also those move vars are static/global - there seems to be a lot of "new" going on.

Not shown any output? How long does it run before crashing? Adding CrashReport?
 
Code:
    if(nextMove)
      Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
Just an observation. You haven't parenthesized the scope of the if statement and therefore only the first .print is inside the if. This means you will print "\n nextMove = " if nextMove is not null. But you will always print (*nextMove).x.
It should, presumably, be:
Code:
    if(nextMove) {
      Serial.print("\n nextMove = ");
      Serial.print((*nextMove).x);
    }

Pete
 
Last edited:
Thanks.... The oneliner if was the fault I have been banging my head for 2 days at this boildown.... feeling stupid now... But this means my error is else where.... Will continiue digging... TNKS!!
 
Thanks.... The oneliner if was the fault I have been banging my head for 2 days at this boildown.... feeling stupid now... But this means my error is else where.... Will continiue digging... TNKS!!

Good catch Pete! ... missed the double statement line in reading
 
And another observation
Code:
    nextMove = NULL;

    if(nextMove)  
      Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
Even if this if statement is parenthesized, this will never print anything because you set nextMove to NULL before the if statement.

Pete
 
As an exercise, I recommend you do two things:

a) explain in words exactly what "moveXY _nextMove = *new moveXY();" does

b) explain when one should use the "delete" operator
 
Finally got on the right track!
Because I used new bufStruct() when adding to the buffer (in a separate class reading of serial and taking the buffer as reference) I never realised them being temporary. And reading about it brought my attention to be able to use the copyfunction of the default generated constructor of structs.
I was using:
Code:
    old = active;
    active = upcoming;
    upcoming= &(commandBuffer.shift()));
The next iteration of the loop will have destroyed the object pointed to by upcoming making (*active).data return garbage.


Code:
delete old; 
    old = active;
    active = upcoming;
    upcoming= new bufStruct((commandBuffer.shift()));
By using new above I no longer have a temporary and am therefore responsible for cleanup - hence "delete old"

Hope this helps someone!
 
Last edited:
Working on something else @KurtE discovered that the NULL Ptr write trap/fault extends to bytes 0-31 and covers both READ and WRITE to those addresses.

That explains the fault when this nextMove pointer was NULL :: Serial.print((*nextMove).x);
 
Working on something else @KurtE discovered that the NULL Ptr write trap/fault extends to bytes 0-31 and covers both READ and WRITE to those addresses.

That explains the fault when this nextMove pointer was NULL :: Serial.print((*nextMove).x);

Yes, the mpu region is 32 bytes minimum.
There is ITCM at 0 anyway, so it is OK.
One could go ahead and cover the whole ITCM (for writes).. i don't think anybody uses self-modyfing code today. And if its really needed, the trap can be disabled.
 
> upcoming= new bufStruct((commandBuffer.shift()));

As with malloc(), one should always check that "new" succeeded.
 
Status
Not open for further replies.
Back
Top