arduino-libraries / MKRGSM

GNU Lesser General Public License v2.1
55 stars 51 forks source link

Externalize URCs handler with a callback #16

Closed FrancMunoz closed 6 years ago

FrancMunoz commented 6 years ago

Hi!

EDITED: Made some changes to improve my suggestion.

I'd like to make a suggestion. I've been working in a large project and need to know for example when a SMS is received instantly without have to pooling if sms is available (that is very slow). So I've created a callback in ModemClass to receive all URCs in my code and parse if necessary.

So in Modem.h added:

void (*urcCallback)(const String& urc);

and in Modem.cpp added the call after urcHandlers:

void ModemClass::poll()
{
  while (_uart->available()) {
    char c = _uart->read();
    _uartMillis=millis();

    if (_debug) {
      Serial.write(c);
    }

    _buffer += c;

    switch (_atCommandState) {
      case AT_COMMAND_IDLE:
      default: {

        if (_buffer.startsWith("AT") && _buffer.endsWith("\r\n")) {
          _atCommandState = AT_RECEIVING_RESPONSE;
          _buffer = "";
        }  else if (_buffer.endsWith("\r\n")) {
          _buffer.trim();

          if (_buffer.length()) {
            for (int i = 0; i < MAX_URC_HANDLERS; i++) {
              if (_urcHandlers[i] != NULL) {
                _urcHandlers[i]->handleUrc(_buffer);
              }
            }
            if(urcCallback!=NULL) urcCallback(_buffer);
          }

          _buffer = "";
        }

        break;
      }
...

So in code could receive SMS notifications. The only issue is the use of flags during the callback to be executed at main loop, because if an operation is done in the callback all seems to fail due the design of the modem code (that I think is nice).

So:

bool isSMS=false;

void handleUrc(const String& urc) {
    Serial.print("URC: ");
    Serial.println(urc);
    if(urc.startsWith("+CMTI")) {
        isSMS=true;
    }
}

void setup() {
    // Setup GSM... 
    MODEM.urcCallback=handleUrc;
    MODEM.send("AT+CNMI=2,1");  // Enables URC SMS
}

void loop() {
    if(isSMS) {
        isSMS=false;
        //checkSMS();
    }
}

This will give more power and control to final coding.

What do you think?

Thank you!

sandeepmistry commented 6 years ago

@FrancMunoz thank you for proposing this. At this time I would prefer to leave this out.

FrancMunoz commented 6 years ago

Hi @sandeepmistry! I see it could be complicated and could generate some problems but improves control on final coding. I would use ModemClass::addUrcHandler because I would like to use your future improvements.

Thank you!