Serial stream StateMachine next state issue

Tsjompie commented 4 years ago

Dear Automaton users,

I'm not sure whether I'm allowed to ask questions like below, but as I'm kind of stuck, I'll try. You can, or I will, remove when this doesn't belong here...

I'm building a SM to read a serial stream from a Nextion TFT screen: the current result is the buffer provides me the correct string from the serial stream, like the P2 or P1 shown below. That buffer still to be passed through to the oncommand.push( lookup( 0, commands ) ); to allow me to run the callback function. This SM is mostly based on the command State Machine as TinkerSpy provided (so much thanks for that!), though to fully understand I used the SM Wolkendek site to build be a complete new SM.

The trace looks as expected:

396588 Switch NEXTION@5A5 from READSTREAM to SEND on EVT_EOL (1 cycles in 14 ms)
396609 Switch NEXTION@5A5 from SEND to IDLE on ELSE (1 cycles in 1 ms)
400979 Switch NEXTION@5A5 from IDLE to READSTREAM on EVT_INPUT (3405 cycles in 4353 ms)
case EVT_EOL    P2   buffer
401006 Switch NEXTION@5A5 from READSTREAM to SEND on EVT_EOL (1 cycles in 19 ms)
401028 Switch NEXTION@5A5 from SEND to IDLE on ELSE (1 cycles in 1 ms)
401656 Switch NEXTION@5A5 from IDLE to READSTREAM on EVT_INPUT (421 cycles in 611 ms)
case EVT_EOL    P2   buffer
401676 Switch NEXTION@5A5 from READSTREAM to SEND on EVT_EOL (1 cycles in 14 ms)
401697 Switch NEXTION@5A5 from SEND to IDLE on ELSE (1 cycles in 1 ms)
402258 Switch NEXTION@5A5 from IDLE to READSTREAM on EVT_INPUT (425 cycles in 543 ms)
case EVT_EOL    P1   buffer
402284 Switch NEXTION@5A5 from READSTREAM to SEND on EVT_EOL (1 cycles in 19 ms)
402306 Switch NEXTION@5A5 from SEND to IDLE on ELSE (1 cycles in 2 ms)
402734 Switch NEXTION@5A5 from IDLE to READSTREAM on EVT_INPUT (264 cycles in 411 ms)

My interpretation is the SM passes the SEND state based on the EVT_EOL code, but the ENT_SEND code does not execute. I would expect the "serial println" below to show in the serial debug window too:

    case ENT_SEND:
      Serial.println("ENT_SEND ENT_SEND ENT_SEND ENT_SEND ENT_SEND ");
      Serial.print(buffer);
      oncommand.push( lookup( 0, commands ) );
      return;

For full reference I attached the ccp and hpp to this "Please help me" request. I'm sorry to have so many serial.prints in the cpp, as that is the way I tried to debug this...

Is there someone able to point out what I miss here? Why does it (look like?) the SEND states passes, but the code assigned to that state doesn't show in the debug window? Any help/input/consideration would be highly appreciated.

Thank you,

Tsjompie

Atm_Nextion.zip

tinkerspy commented 4 years ago

Please don't attach code in zip files, just link directly to a gist. Much easier for the reader.

I've recently discovered a bug in the Atm_command machine. The EVT_EOL event should come before the EVT_INPUT event in the state table. The way it was everything works fine when typing commands on the terminal. But when a continuous stream of data is sent (Copy/paste a few 100 lines in the putty terminal program) the EVT_EOL event won't be triggered because the SM is too busy reading characters to check the end-of-line condition.

These are the relevant code sections:

  const static state_t state_table[] PROGMEM = {
    /*                  ON_ENTER    ON_LOOP    ON_EXIT  EVT_EOL   EVT_INPUT   ELSE */
    /* IDLE     */            -1,        -1,        -1,        -1, READCHAR,    -1,
    /* READCHAR */  ENT_READCHAR,        -1,        -1,      SEND, READCHAR,    -1,
    /* SEND     */      ENT_SEND,        -1,        -1,        -1,       -1,  IDLE,
  };

  enum { EVT_EOL, EVT_INPUT, ELSE };

Perhaps this is your problem as well.

edit: Fixed wrong order in state table comment!

Tsjompie commented 4 years ago

Update 20191214: Nailed it! Update will follow shortly.

Some more details around my ongoing fix:

This is the state table I created:

  const static state_t state_table[] PROGMEM = {
    /*                     ON_ENTER  ON_LOOP  ON_EXIT  EVT_CHAR_IN  EVT_CHAR_START  EVT_CHAR_END  EVT_CMD_SEND  EVT_BUFFER_ISSUE        ELSE */
    /* CONNECTION */ ENT_CONNECTION,      -1,      -1,     DATA_IN,             -1,           -1,           -1,               -1,         -1,
    /*    DATA_IN */             -1,      -1,      -1,          -1,       CMD_READ,           -1,           -1,               -1,         -1,
    /*   CMD_READ */             -1,      -1,      -1,          -1,             -1,    CMD_CLOSE,           -1,       CONNECTION,         -1,
    /*  CMD_CLOSE */             -1,      -1,      -1,          -1,             -1,           -1,     CMD_SEND,               -1,         -1,
    /*   CMD_SEND */             -1,      -1,      -1,          -1,             -1,           -1,           -1,               -1, CONNECTION,
  };

When setting all the returns to 1 this follows the steps to:

determine the availability of the serial connection
check for the incoming start character
put the command into a buffer, untill the end character passes
possibly add (e.g. "\0") before matching the command on the cmd_list (not sure yet, but based on the command State Machine I expect to need that string closure).
do some buffersize error handling (when no end character and therefor more than expected characters come in)

What stil needs to be accomplished is that I cannot get from the read to the send state. For unapparent reasons I cannot get the return set correctly here:

    case EVT_CHAR_END:
        Serial.println("EVT_CHAR_END");
        Serial.print(readChar);
        Serial.println("\t readChar");
        unsigned long tmr_1 = millis();

        while(stream->available()){ // 
          if((millis() - tmr_1) > 100){    // Waiting... But not forever...... Allow the serial port to deliver char
            readChar = stream->read();
            buffer[bufptr++] = readChar;
            Serial.print(buffer);
            Serial.println("\t buffer collecting");
          } // END if((millis() - tmr_1)
        } //END While or IF

        if( readChar == endChar ) {
          return 1;
        }
        Serial.println("Does this pass?");
      return 0;

I'll update this items (with the final code) once that is completed. This might help others to create their own state machine 👍

Tsjompie

My current ino:

#include <Automaton.h>
#include <nexConnect.h>
#include <nexData.h>
#include <Atm_nexSerial.hpp>

#pragma GCC diagnostic warning "-Wunused-variable"
#pragma GCC diagnostic ignored "-Wunused-result"
#pragma GCC diagnostic ignored "-Wunused-parameter"

enum { page1, page2, page4, NuttonL };
// const char cmdlist[] = "get low read aread awrite mode_input mode_output mode_pullup";
const char nexSerialStreamlist[] = "<P1> <P2> <p4> <L1> <L2> ";          //1 2 3 4 5 6 7 8
char nexSerialStream_buffer[10];
Atm_nexSerial nexSerialStream;

Atm_timer teller;

void timer_callback( int idx, int v, int up ) {
  // Something to do when the timer goes off
  Serial.print(idx);
  Serial.print(" - ");
  Serial.print(v);
  Serial.print(" - ");
  Serial.print(up);
  Serial.print(" - ");
  Serial.println("timer_callback");
}
void setup() {

Serial.begin( 38400 );
nexSerial.begin( 38400 );

  teller.begin( 8000 )
    .repeat( -1 )
    .onTimer( timer_callback )
    .start();

//COMMAND MACHINES
    nexSerialStream.begin( nexSerial, nexSerialStream_buffer, sizeof( nexSerialStream_buffer ) )
    .list( nexSerialStreamlist )
    .trace( Serial )
    .onCommand( nexSerialStream_callback );

}

void loop() {
  automaton.run();
}

void nexSerialStream_callback( int idx, int v, int up ) {
Serial.println("CallBack executed nexSerialStream");

Serial.print(idx);
Serial.print("\t IDX \t");
Serial.print(v);
Serial.print("\t v \t");
Serial.print(up);
Serial.println("\t up");
}

My current hpp:

#pragma once

#include <Automaton.h>

class Atm_nexSerial: public Machine {

 public:
  enum { CONNECTION, DATA_IN, CMD_READ, CMD_CLOSE, CMD_SEND }; // STATES
  enum { EVT_CHAR_IN, EVT_CHAR_START, EVT_CHAR_END, EVT_CMD_SEND, EVT_BUFFER_ISSUE, ELSE }; // EVENTS
  Atm_nexSerial& begin( Stream& stream, char buffer[], int size );
  Atm_nexSerial& trace( Stream & stream );
  Atm_nexSerial& onCommand( atm_cb_push_t callback, int idx = 0 );
  Atm_nexSerial& list( const char* cmds );
  Atm_nexSerial& trigger( int event );
  int state( void );
  Atm_nexSerial& char_in( void );
  Atm_nexSerial& char_start( void );
  Atm_nexSerial& char_end( void );
  Atm_nexSerial& cmd_close( void );
  Atm_nexSerial& cmd_send( void );
  Atm_nexSerial& buffer_issue( void );

    char* arg( int id );
  int lookup( int id, const char* cmdlist );

 private:
  enum { ENT_CONNECTION }; // ACTIONS
  atm_connector oncommand;
  Stream* stream;
  char* buffer;
  int bufsize, bufptr;
  char endChar, startChar, readChar;    //eol, 
  bool buffull;
  bool endCharPass;
  const char* commands;
  int event( int id ); 
  void action( int id );
};

And the current cpp:

#include <Automaton.h>
#include <nexConnect.h>
#include <nexData.h>
#include <Atm_nexSerial.hpp>

#pragma GCC diagnostic warning "-Wunused-variable"
#pragma GCC diagnostic ignored "-Wunused-result"
#pragma GCC diagnostic ignored "-Wunused-parameter"

enum { page1, page2, page4, NuttonL };
// const char cmdlist[] = "get low read aread awrite mode_input mode_output mode_pullup";
const char nexSerialStreamlist[] = "<P1> <P2> <p4> <L1> <L2> ";          //1 2 3 4 5 6 7 8
char nexSerialStream_buffer[10];
Atm_nexSerial nexSerialStream;

Atm_timer teller;

void timer_callback( int idx, int v, int up ) {
  // Something to do when the timer goes off
  Serial.print(idx);
  Serial.print(" - ");
  Serial.print(v);
  Serial.print(" - ");
  Serial.print(up);
  Serial.print(" - ");
  Serial.println("timer_callback");
}
void setup() {

Serial.begin( 38400 );
nexSerial.begin( 38400 );

  teller.begin( 8000 )
    .repeat( -1 )
    .onTimer( timer_callback )
    .start();

//COMMAND MACHINES
    nexSerialStream.begin( nexSerial, nexSerialStream_buffer, sizeof( nexSerialStream_buffer ) )
    .list( nexSerialStreamlist )
    .trace( Serial )
    .onCommand( nexSerialStream_callback );

}

void loop() {
  automaton.run();
}

void nexSerialStream_callback( int idx, int v, int up ) {
Serial.println("CallBack executed nexSerialStream");

Serial.print(idx);
Serial.print("\t IDX \t");
Serial.print(v);
Serial.print("\t v \t");
Serial.print(up);
Serial.println("\t up");
}

Tsjompie commented 4 years ago

Let me close this one for now and share my machine once completely done.

Tsjompie

tinkerspy / Automaton

Serial stream StateMachine next state issue #69