tinkerspy / Automaton

Reactive State Machine Framework for Arduino
https://github.com/tinkerspy/Automaton/wiki
MIT License
374 stars 63 forks source link

Triggering event from inside Machine::action causes silent crash on Arduino Mega #68

Closed 00benallen closed 4 years ago

00benallen commented 5 years ago

Here's the offending code:

void Atm_machine::action( int id ) {
  switch ( id ) {
    case LP_STATE:
      otherMachine.event_function();
      return;
    case OTHER_STATE:
      return;
  }
}

My Arduino just silently restarts when I do this.

tinkerspy commented 5 years ago

In general, you shouldn't be calling routines from one Machine object inside another machine object. I also wonder what event_function's relation to event() is.

If you do things you shouldn't do, it's not that surprising that things start crashing.

klaudiusz223 commented 5 years ago

I do almost the same, but i use pointer to another machine. Event method in another machine is declared as public.

void Atm_machine::action( int id ) {
  switch ( id ) {
    case LP_STATE:
      otherMachine->event_function();
      return;
    case OTHER_STATE:
      return;
  }
}
00benallen commented 5 years ago

@tinkerspy I did come to the same conclusion, however, I wonder if it would be possible to have a warning/error message.

Part of the problem was I couldn't find very good documentation on inter-machine communication in the first place and had to read a lot of source code to figure out a better approach.

With neither an error message or good docs, my developer experience was "why does my arduino keep restarting?"

tinkerspy commented 5 years ago

On microcontrollers warnings and messages are not really possible. You can't really start outputting strings on some usb/serial port in the hope that somebody's watching. An mcu doesn't really have a user interface until you write one.

Machine communication is documented in the Wiki: https://github.com/tinkerspy/Automaton/wiki/Machine-communication

Even more documentation can be found in the second Machine building tutorial, when you can read how to create a connector (the outgoing part of event communication) that can be used to trigger other machines:

https://github.com/tinkerspy/Automaton/wiki/Machine-building-tutorial-2

You see, the idea is that machines call each other's triggers to communicate.

00benallen commented 5 years ago

The machine communication part of the wiki doesn’t explain anything about connectors, and the machine building example 2 doesn’t explain everything about using push, since it’s just 1 example and it’s a simple one.

I get what you’re saying about warnings, although I do use some sensor libraries which do output warnings in certain situations.

I wonder if the fact that I’m having this problem is an indication that the docs need some additions? I get it now but it wasn't obvious until I spend a while reading your source code.

On Tue, Nov 26, 2019 at 5:21 PM Tinkerspy notifications@github.com wrote:

On microcontrollers warnings and messages are not really possible. You can't really start outputting strings on some usb/serial port in the hope that somebody's watching. An mcu doesn't really have a user interface until you write one.

Machine communication is documented in the Wiki: https://github.com/tinkerspy/Automaton/wiki/Machine-communication

Even more documentation can be found in the second Machine building tutorial, when you can read how to create a connector (the outgoing part of event communication) that can be used to trigger other machines:

https://github.com/tinkerspy/Automaton/wiki/Machine-building-tutorial-2

You see, the idea is that machines call each other's triggers to communicate.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tinkerspy/Automaton/issues/68?email_source=notifications&email_token=ABQVBWWTOOXT5YOLT27PL7TQVWOODA5CNFSM4JRRCF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFHUG4Q#issuecomment-558842738, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQVBWVBGIDAWHZV5WS4QUTQVWOODANCNFSM4JRRCF4Q .

tinkerspy commented 5 years ago

It's not a trivial concept. If you've just freshly figured it out, perhaps you can explain it better than me. If you write a text (in markdown preferable, perhaps as a github gist) I'll add it to the wiki.

00benallen commented 5 years ago

Cool okay

00benallen commented 5 years ago

@tinkerspy Here's my first draft, feel free to edit!

https://gist.github.com/00benallen/a94b0919bcda986d90041c287bd9539b

00benallen commented 4 years ago

@tinkerspy bump

00benallen commented 4 years ago

@rawtaz why the thumbs down?

rawtaz commented 4 years ago

@00benallen Nothing serious, but the author of this framework is a busy man and he's developing this library free of charge and in his free time. Bumping a thread just two days after your last message is not constructive - he will look at your contribution when he can, there's no need to remind him about it. It also pollutes the thread with that type of messages. In other words, give him some slack. That's why I thumbed down :)

tinkerspy commented 4 years ago

Thanks, 00benallen, it's a lot better than my text. I think I'll just replace my text with it if that's alright with you. (I'll credit you, of course)

00benallen commented 4 years ago

Yeah sure, I'm happy to have helped. Thanks for being open-minded! :)

tinkerspy commented 4 years ago

It's on line. Thanks again!

00benallen commented 4 years ago

@tinkerspy well I guess this is solved, I'll close this now

Tsjompie commented 4 years ago

Gents,

What would be my best alternative passing a textstring using this inter-machine communication? The current option seems to use int's only void Machine::push( atm_connector connectors[], int id, int sub, int v, int up ) Or is my best alternative to use a global variable for that? Thanks :)

tinkerspy commented 4 years ago

Call a method from the callback that returns the string. When the callback is triggered it requests the string from the machine that called it.

Rgrdz, Tinkerspy

Tsjompie commented 4 years ago

Thanks!

Tsjompie commented 4 years ago

OOps, still haven't been able to build as suggested above and kindly request for a little more directions on that (sorry).

From the ino, in which I have this:

nexSerialStream.begin( nexSerial, nexSerialStream_buffer, sizeof( nexSerialStream_buffer ) )
    .list( nexSerialStreamlist )
    .trace( Serial )
    .onRxcommand( nexSerialStream_callback );
}

I'm able to execute a function onrxcommand.push( lookup( 0, commands ) ); and can run code based on what comes in via the Rx. However I still fail in injecting a string like n0.val=88 to use the ENT_SEND_SET or get n0.val using ENT_SEND_GET.

The first function below works, do I need to

add the method from the callback that returns the string

in the second ON_TXCOMMAND one?

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onRxcommand( atm_cb_push_t callback, int idx ) { // onPush( connectors, ON_RXCOMMAND, 0, 1, 1, callback, idx ); onrxcommand.set( callback, idx); return *this; } Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onTxcommand( atm_cb_push_t callback, int idx ) { onPush( connectors, ON_TXCOMMAND, 0, 1, 1, callback, idx );

STRING RETURN FUNCTION HERE?

return *this; }

The state table looks like:

const static state_t state_table[] PROGMEM = {
    /*                      ON_ENTER  ON_LOOP  ON_EXIT  EVT_STREAM_START  EVT_START_TXT  EVT_PROCESS_TXT  EVT_START_INT  EVT_PROCESS_INT  EVT_START_CMD  EVT_PROCESS_CMD  EVT_CLOSE_CHARS  EVT_STREAM_CLOSE         ELSE */
    /*  CONNECTION */ ENT_CONNECTION,      -1,      -1,         START_RX,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,          -1,
    /* TX_RECEIVER */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,  CONNECTION,
    /*    SEND_SET */   ENT_SEND_SET,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1, TX_RECEIVER,
    /*    SEND_GET */   ENT_SEND_GET,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,        START_RX,               -1,          -1,
    /*    START_RX */             -1,      -1,      -1,               -1,       GET_TXT,              -1,       GET_INT,              -1,       GET_CMD,              -1,              -1,               -1,          -1,
    /*     GET_TXT */             -1,      -1,      -1,               -1,            -1,         FIN_TXT,            -1,              -1,            -1,              -1,              -1,               -1,          -1,
    /*     GET_INT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,         FIN_INT,            -1,              -1,              -1,               -1,          -1,
    /*     GET_CMD */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,         FIN_CMD,              -1,               -1,          -1,
    /*     FIN_TXT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     FIN_INT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     FIN_CMD */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     CONFIRM */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,  CONNECTION,
  };

Would you mind giving me a little more direction in how to structure/enable this?

I would expect to use some kind of parameter in one of the cpp functions to "hand over" the string requesting for the number object on the screen. Would that be this: Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onTxcommand? Still haven't figured out how to actually deal with that...

Thanks for any input around this, helping me further... Happy X-mas days too.

Tsjompie

The cpp looks as follows:

#include "Atm_nexSerial_RX_TX.hpp"
#include <Atm_nexSerial.hpp>
#include <nexData.h>

nexData nexData;

/* Add optional parameters for the state machine to begin()
 * Add extra initialization code
 */

#define NEX_RET_STRING_HEAD                 (0x70)
#define NEX_RET_NUMBER_HEAD                 (0x71)
#define NEX_RET_CMD_HEAD                    (0x3c)
#define NEX_RET_CMD_CLOSE                   (0x3e)
#define NEX_RET_CMD_FINISHED                (0x01)

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::begin( Stream& stream, char buffer[], int size ) {
  // clang-format off
  const static state_t state_table[] PROGMEM = {
    /*                      ON_ENTER  ON_LOOP  ON_EXIT  EVT_STREAM_START  EVT_START_TXT  EVT_PROCESS_TXT  EVT_START_INT  EVT_PROCESS_INT  EVT_START_CMD  EVT_PROCESS_CMD  EVT_CLOSE_CHARS  EVT_STREAM_CLOSE         ELSE */
    /*  CONNECTION */ ENT_CONNECTION,      -1,      -1,         START_RX,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,          -1,
    /* TX_RECEIVER */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,  CONNECTION,
    /*    SEND_SET */   ENT_SEND_SET,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1, TX_RECEIVER,
    /*    SEND_GET */   ENT_SEND_GET,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,        START_RX,               -1,          -1,
    /*    START_RX */             -1,      -1,      -1,               -1,       GET_TXT,              -1,       GET_INT,              -1,       GET_CMD,              -1,              -1,               -1,          -1,
    /*     GET_TXT */             -1,      -1,      -1,               -1,            -1,         FIN_TXT,            -1,              -1,            -1,              -1,              -1,               -1,          -1,
    /*     GET_INT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,         FIN_INT,            -1,              -1,              -1,               -1,          -1,
    /*     GET_CMD */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,         FIN_CMD,              -1,               -1,          -1,
    /*     FIN_TXT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     FIN_INT */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     FIN_CMD */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,          CONFIRM,          -1,
    /*     CONFIRM */             -1,      -1,      -1,               -1,            -1,              -1,            -1,              -1,            -1,              -1,              -1,               -1,  CONNECTION,
  };
  // clang-format on
  Machine::begin( state_table, ELSE );
  this->stream = &stream;
  this->buffer = buffer;
  bufsize = size;
  bufptr = 0;
  buffull = 0;
  // separatorChar = " ";
  startChar = '<';
  endChar = '>';
  endCharPass = 0;
  return *this;       
}

/* Add C++ code for each internally handled event (input) 
 * The code must return 1 to trigger the event
 */

int Atm_nexSerial_RX_TX::event( int id ) {
  switch ( id ) {
    case EVT_STREAM_START:
        if(stream->available() >= 3) {
          bufptr = 0;
            readChar = stream->read();
            return 1;
        }
        return 0;    
    case EVT_START_TXT:
            if (readChar == NEX_RET_STRING_HEAD) {
                return 1;
            }
        return 0;
    case EVT_PROCESS_TXT:
      return 0;
    case EVT_START_INT:
            if (readChar == NEX_RET_NUMBER_HEAD) {
                return 1;
            }
        return 0;
    case EVT_PROCESS_INT:
      return 0;
    case EVT_START_CMD:
        while (stream->available() > 0) {
            if (readChar == NEX_RET_CMD_HEAD) {
                buffer[bufptr++] = readChar;
                return 1;
            }
        } 
        return 0;
    case EVT_PROCESS_CMD:
        while (stream->available() > 0) {
            readChar = stream->read();
                buffer[bufptr++] = readChar;
               if (readChar == NEX_RET_CMD_CLOSE) {
                onrxcommand.push( lookup( 0, commands ) );
                return 1;
                }
        }
        return 0;
    case EVT_CLOSE_CHARS:
      for( int i = 0; i < sizeof(buffer);  ++i ) {
      buffer[i] = (char)0; }
      bufptr = 0;
      return 1;
    case EVT_STREAM_CLOSE:
      return 1;
  }
  return 0;
}

/* Add C++ code for each action
 * This generates the 'output' for the state machine
 *
 * Available connectors:
 *   push( connectors, ON_RXCOMMAND, 0, <v>, <up> );
 *   push( connectors, ON_TXCOMMAND, 0, <v>, <up> );
 */

void Atm_nexSerial_RX_TX::action( int id ) {
  switch ( id ) {
    case ENT_CONNECTION:
      return;
    case ENT_SEND_SET:
      delay(10);
      push( connectors, ON_RXCOMMAND, 0, 0, 0 );
      return;
    case ENT_SEND_GET:
      return;
  }
}

/* Optionally override the default trigger() method
 * Control how your machine processes triggers
 */

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::trigger( int event ) {
  Machine::trigger( event );
  return *this;
}

/* Optionally override the default state() method
 * Control what the machine returns when another process requests its state
 */

int Atm_nexSerial_RX_TX::state( void ) {
  return Machine::state();
}

/* Nothing customizable below this line                          
 ************************************************************************************************
*/

/* Public event methods
 *
 */

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::stream_start() {
  trigger( EVT_STREAM_START );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::start_txt() {
  trigger( EVT_START_TXT );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::process_txt() {
  trigger( EVT_PROCESS_TXT );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::start_int() {
  trigger( EVT_START_INT );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::process_int() {
  trigger( EVT_PROCESS_INT );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::start_cmd() {
  trigger( EVT_START_CMD );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::process_cmd() {
  trigger( EVT_PROCESS_CMD );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::close_chars() {
  trigger( EVT_CLOSE_CHARS );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::stream_close() {
  trigger( EVT_STREAM_CLOSE );
  return *this;
}

/*
 * onRxcommand() push connector variants ( slots 1, autostore 0, broadcast 0 )
 */

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onRxcommand( Machine& machine, int event ) {
  onPush( connectors, ON_RXCOMMAND, 0, 1, 1, machine, event );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onRxcommand( atm_cb_push_t callback, int idx ) {
  // onPush( connectors, ON_RXCOMMAND, 0, 1, 1, callback, idx );
  onrxcommand.set( callback, idx);
  return *this;
}

/*
 * onTxcommand() push connector variants ( slots 1, autostore 0, broadcast 0 )
 */

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onTxcommand( Machine& machine, int event ) {
  onPush( connectors, ON_TXCOMMAND, 0, 1, 1, machine, event );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::onTxcommand( atm_cb_push_t callback, int idx ) {
  onPush( connectors, ON_TXCOMMAND, 0, 1, 1, callback, idx );
  return *this;
}

/* State trace method
 * Sets the symbol table and the default logging method for serial monitoring
 */

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::trace( Stream & stream ) {
  Machine::setTrace( &stream, atm_serial_debug::trace,
    "NEXSERIAL_RX_TX\0EVT_STREAM_START\0EVT_START_TXT\0EVT_PROCESS_TXT\0EVT_START_INT\0EVT_PROCESS_INT\0EVT_START_CMD\0EVT_PROCESS_CMD\0EVT_CLOSE_CHARS\0EVT_STREAM_CLOSE\0ELSE\0CONNECTION\0TX_RECEIVER\0SEND_SET\0SEND_GET\0START_RX\0GET_TXT\0GET_INT\0GET_CMD\0FIN_TXT\0FIN_INT\0FIN_CMD\0CONFIRM" );
  return *this;
}

Atm_nexSerial_RX_TX& Atm_nexSerial_RX_TX::list( const char* cmds ) {
    Serial.println("LIST");
    Serial.println(cmds);
  commands = cmds;
  return *this;
}

char* Atm_nexSerial_RX_TX::arg( int id ) {
  int cnt = 0;
  int i;
  if ( id == 0 ) return buffer;
  for ( i = 0; i < bufptr; i++ ) {
    if ( buffer[i] == '\0' ) {
      if ( ++cnt == id ) {
        i++;
        break;
      }
    }
  }
  return &buffer[i];
}

int Atm_nexSerial_RX_TX::lookup( int id, const char* cmdlist ) {
  Serial.println("Lookup passed");
  int cnt = 0;
  char* arg = this->arg( id );
  char* a = arg;
  while ( cmdlist[0] != '\0' ) {
    while ( cmdlist[0] != '\0' && toupper( cmdlist[0] ) == toupper( a[0] ) ) {
      cmdlist++;
      a++;
    }
    if ( a[0] == '\0' && ( cmdlist[0] == ' ' || cmdlist[0] == '\0' ) ) return cnt;
    while ( cmdlist[0] != ' ' && cmdlist[0] != '\0' ) cmdlist++;
    cmdlist++;
    a = arg;
    cnt++;
  }
  return -1;
}

Based on the lookup as mentioned above, I'm able to execute a function belonging to a

the callback to my self created onrxcommand.push( lookup( 0, commands ) ); works. This is equal to the original atm_command functionality, with other names though.

My ino

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

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

enum nexActions { page1, page2, page4, NuttonL };
// const char cmdlist[] = "get low read aread awrite mode_input mode_output mode_pullup";
const char nexSerialStreamlist[] = "<P0> <P1> <P2> <P3> <P4> <L1> <L2> <L3> <L4> <L5> <L6> <L7> <L8> ";          //1 2 3 4 5 6 7 8
char nexSerialStream_buffer[6];
Atm_nexSerial_RX_TX nexSerialStream;

Atm_timer teller;

void setup() {

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

  teller.begin( 10000 )
    .repeat( -1 )
    .onTimer( timer_callback )
    // .onTimer( nexSerialTX )
    .trace( Serial )
    .start();

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

String nexSerialTX( ) {
  Serial.println("n0.val=8");
  String a = "n0.val=88";
  return a;
}

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");

  //nexSerialStream.onTxcommand( nexSerialTX );
}

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

void nexSerialStream_callback( int idx, int v, int up ) {
    // Serial.print(v);
    // Serial.println("\t Switch ID after screen press");
    switch ( v ) {
        case 0:       //Load page 0 Disclaimer
        Serial.println("P0 Disclaimer page");
        break;

        case 1:       //Load page 1 (btn page)
        Serial.println("P1 BTn page");
        break;

        case 2:       //Load page 1 (btn page)
        Serial.println("P2 Matrix page");
        break;

        case 3:       //Load page 3 (SETTINGS page)
        Serial.println("P3 SETTINGS page");
        break;
    }
}