Go Down

Topic: variables living a life of their own (Read 162 times) previous topic - next topic

Newmanx

If you look at the picture from a scope, you'll see a debug output for my Arduino program. Blue (waveform 3) and green (waveform 4) are the the interesting signals at the moment.



According to my understanding of the code I wrote, the GREEN signal has no right to go LOW before the BLUE one does. And yet sometimes it does exactly that (marked area in the picture).

I must be making some stupid rookie mistake, but I have no idea where and what.

The following struct holds the data while it's being decoded:

Code: [Select]

struct TDecodedStep {
#define STEP_MESSAGE_SIZE 6
    byte        axis;
    bool        dir;
    uint32_t    delay;

    TDelay      delUn;

    byte        count;
    bool        ready;
};


The following part decodes the data and sets both signals HIGH and decodedStep.ready = true; when a message has been decoded in full.


Code: [Select]
void decodeSerial(const byte b) {

    NeoSerial.write(b);

    if (decodedStep.count == 0) {
        byte header = b >> 4;
        if (header == 0xB) {
            bitClear(PIN3P, PIN3B); // Green TEST
            decodedStep.axis = header & 0x7;
            decodedStep.dir = header & 0x8;
            decodedStep.count++;
        } else {
            NeoSerial.print("EH\n"); // report error: incorrect header
        }
    } else if (decodedStep.count < STEP_MESSAGE_SIZE - 1) { // until we've approached the end (before the delimiter)
        //*((byte*)&(decodedStep.delay) + decodedStep.count - 1) = b; // why doesn't this work???
        decodedStep.delUn.b[decodedStep.count - 1] = b; // building the variable. -1 for the first message byte.
        decodedStep.count++;
    } else if (decodedStep.count == STEP_MESSAGE_SIZE - 1) {
        if (b == '\n') {                // message properly ended with a delimiter
            decodedStep.delay = decodedStep.delUn.i;

            NeoSerial.print("\n\r"); // TEST
            NeoSerial.print(decodedStep.axis); // TEST
            NeoSerial.print(decodedStep.dir); // TEST
            NeoSerial.print(decodedStep.delay); // TEST
            NeoSerial.print("\n\n"); // TEST

            bitSet(PIN3P, PIN3B); // Green TEST
            bitSet(PIN2P, PIN2B); // Blue TEST
            decodedStep.ready = true;
            decodedStep.count = 0;
        } else {
            decodedStep.axis = 0;
            decodedStep.dir = 0;
            decodedStep.delay = 0;
            decodedStep.count = 0;

            NeoSerial.print("EN\n"); // report error: no newline delimiter
        }
    }

}


Then, when the executive routine (updateSteppers) is ready, it takes the decodedStep contents, and sets decodedStep.ready = false; so the next message can be decoded. At the same time, it sets BLUE LOW. So BLUE cannot be LOW without decodedStep.ready = false;.

Code: [Select]

void updateSteppers() {
    unsigned long int currentMicros = micros();

    switch(stepState) {
    case kStepIdle:         // READY TO MAKE A NEW STEP
        if (decodedStep.ready) {
            // Copy the decoded step parameters
            currentAxis = decodedStep.axis;
            currentDir = decodedStep.dir;
            currentDelay = decodedStep.delay;

            // Error. Too long delay between pulses. The pulse should've already ended at the time it just started to get processed
            if ((currentMicros - lastPulseEnd[currentAxis]) >= (currentDelay - pulPulseDuration)) {
                NeoSerial.print("ED\n");
            }

            // Clear the step container
            decodedStep.axis = 0;
            decodedStep.dir = 0;
            decodedStep.delay = 0;
            decodedStep.ready = false;

            bitClear(PIN2P, PIN2B); // Blue TEST

            stepState = kStepProcessingBegin;
        }
        break;
    case kStepProcessingBegin:  // MAKE THE DIRECTION PULSE IF NECESSARY
        if (lastDir[currentAxis] != currentDir) {   // The direction has changed. We need to update the driver
            lastDir[currentAxis] = currentDir;
            writeDirBit(currentAxis, currentDir);

            lastDirPulse = currentMicros;
            stepState = kStepInsideDirDelay;
        } else {                    // The direction hasn't changed
            stepState = kStepAfterDirDelay;
        }
        break;
    case kStepInsideDirDelay:   // END THE DIRECTION PULSE AFTER A GIVEN TIME, IF THE PULSE WAS NECESSARY
        if(currentMicros - lastDirPulse >= dirPulseDelay) {
            lastDirPulse = 0;
            stepState = kStepAfterDirDelay;
        }
        break;
    case kStepAfterDirDelay:    // WAIT FOR THE DELAY DURATION TO MAKE A MOVEMENT PULSE
        if ((currentMicros - lastPulseEnd[currentAxis]) >= (currentDelay - pulPulseDuration)) {
            writePulBit(currentAxis, LOW);

            lastPulseEnd[currentAxis] = 0;
            lastPulseBegin = currentMicros;
            stepState = kStepInsidePulse;
        }
        break;
    case kStepInsidePulse:      // END THE MOVEMENT PULSE AFTER A GIVEN TIME
        if (currentMicros - lastPulseBegin >= pulPulseDuration) {
            writePulBit(currentAxis, HIGH);

            lastPulseBegin = 0;
            lastPulseEnd[currentAxis] = currentMicros;
            stepState = kStepIdle;
        }
        break;
    }
}


And as I said, decoding of the next message can proceed only when decodedStep.ready = false;:

Code: [Select]

void readSerial() {
    while (NeoSerial.available() > 0) rxBuffer.push(NeoSerial.read());

    if (decodedStep.ready) return; //wait for the processing routine to make use of the last decoded step

    if (operationInterrupted && decodedStep.count == 0) serviceStop();

    //if (rxBuffer.size() >= rxBufferAlmostFull) bitSet(PIN3P, PIN3B);  // Dear remote transmitter, please make a pause
    if (rxBuffer.size() >= rxBufferFull) NeoSerial.print("EF\n");       // Error. Buffer full
    while (rxBuffer.size() > 0) {
        decodeSerial(rxBuffer.pop());
    }
    //if (rxBuffer.size() < rxBufferAlmostEmpty) bitClear(PIN3P, PIN3B);    // We want more RX data
}


So how the heck decoding of the next step starts (GREEN goes LOW) when decodedStep.ready has not been set false; (as seen by BLUE staying HIGH)??

This is somehow related to another issue occurring together.

When I tried using pointers in message decoding (commented out

*((byte*)&(decodedStep.delay) + decodedStep.count - 1) = b;

part in decodeSerial()), the resulting value was wrong in some situations, although correct bytes went in. Using a union, so also delaying modification of decodedStep.delay to the very end, helped with the problem. But it still occurs for decodedStep.axis and decodedStep.dir. They get modified at some point, although they should not be.

So, in general, some weird stuff must happen with my decodedStep struct. But I have no clue what is it.

J-M-L

Hello - Please do not PM me for help,  others will benefit as well if you post your question publicly on the forums.
Bonjour Pas de messages privés SVP, postez dans le forum directement pour que ça profite à tous

arduarn

As already pointed out, posting snippets rarely wins you any brownie points and can make diagnosis impossible. If the code is too large then make a minimal compilable version that exhibits the same behaviour.

That said, based on what you posted, I would guess that the problem is a result of the following:

Code: [Select]
  while (NeoSerial.available() > 0) rxBuffer.push(NeoSerial.read());
  if (decodedStep.ready) return; // ARDUARN: What if we have a message plus a little more in the rxBuffer?
...
  while (rxBuffer.size() > 0) {
    decodeSerial(rxBuffer.pop()); // ARDUARN: Here we decode the message, set ready, but keep processing characters from the next message.
  }


Code: [Select]
  } else if (decodedStep.count == STEP_MESSAGE_SIZE - 1) {
...
      bitSet(PIN3P, PIN3B); // Green TEST
      bitSet(PIN2P, PIN2B); // Blue TEST
      decodedStep.ready = true;
      decodedStep.count = 0; //   ARDUARN: We merrily set count to zero here with ready true.
    } else {
...
    }
  }


Code: [Select]
  if (decodedStep.count == 0) {
...
    if (header == 0xB) {
      bitClear(PIN3P, PIN3B); // ARDUARN: So now green goes low again.
...
    } else {
...
    }
  }


It may be trivially correctable with:
Code: [Select]
  while (rxBuffer.size() > 0 && !decodedStep.ready) {
    decodeSerial(rxBuffer.pop());
  }

...but no warranties from me.

Newmanx

#3
Mar 23, 2019, 01:33 am Last Edit: Mar 23, 2019, 01:36 am by Newmanx
THANK. YOU. Arduarn! Yeah, it was that. So trivial, yet I've wasted so much time with it. That's why I needed a fresh perspective. Thank you very much!

I could've posted the whole sketch, but it wouldn't do much without another which sends the data, which wouldn't work without the third one... I just hoped for someone to do what you did, just look and notice what I've missed. Thank you for the third time! :D

arduarn


Go Up