Cellular: Make ATHandler::cmd_start() virtual

[wajahat-abbas]

Description

Ublox cellular modems, in particular SARA-R4, feature a proprietary power saving mode also known as idle mode. This idle mode is not to be confused with 3GPP Power Saving Mode. It can be enabled or disabled using AT+UPSV command. Once the idle mode is enabled, modem goes to sleep mode if there is an inactivity on serial interface between modem and hostMCU for more than 6 seconds.

The mechanism to wake the modem up from sleep mode is to send a character (e.g. 'A') to the modem before sending the actual AT command. However modem takes a while (~1ms) before it is ready to accept AT commands. This mean that to communicate with the modem while it is asleep, we need to send AT command like this:
Send 'A'
Delay(1)
Send <AT_command>

In order to incorporate the wake-up mechanism, we need to make the ATHandler::cmd_start() virtual so that it can be overridden with our own implementation. This means that modem will always be waken up irrespective of the AT_xxxxxxx class from where function call is made.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

[AriParkkila]

@wajahat-ublox ATHandler::lock() could be a better place to wakeup modem? That is always called before sending a sequence of AT commands to a cellular module. Some commands may take more that 6 seconds but modem probably won't sleep while there is a command still pending. I guess you also need to set time of the last AT traffic in unlock to check if there is less than 6 secs from the latest AT traffic to avoid unnecessary delays.

Could the implementation be similar to ATHandler:::_at_send_delay? That way the time-to-wakeup (6s) and wakeup-delay (1ms) would be given in ATHandler constructor, with default values 0.

[wajahat-abbas]

Thank you for your suggestion. I will surely consider your proposal and get a more through understanding. One thing i want to highlight is that each modem has a different wake-up mechanism e.g. the SARA-U2 requires a pin toggle so in future the wake-up mechanism would be target dependent. I think it would be much easier to manage this vendor specific feature in one place.
I am sharing details regarding this change just so that we have a better understanding. Please refer to the code below:

void UBLOX_ATHandler::cmd_start(const char *cmd)
{
    uint8_t time_out=5; //wait a maximum of 250ms

    if (_idle_mode_status == true) { //idle mode is active
        if ( _wakeup_timer.read_ms() > 5000) { //if more than 5 secs have passed since last TX activity, wake up the modem.
            while(time_out--) {
                ATHandler::cmd_start("A");
                ATHandler::cmd_stop();
                rtos::Thread::wait_until(1); //works without this line as well but i am keeping it for now
                ATHandler::cmd_start("AT");
                ATHandler::cmd_stop();
                ATHandler::resp_start();
                ATHandler::resp_stop();
                if (ATHandler::get_last_error() == NSAPI_ERROR_OK) {
                    break;
                } else {
                    rtos::Thread::wait_until(50);
                }
            }
        }
        _wakeup_timer.reset(); //Any activity on tx will reset the timer. Timer will expire if no activity since last 5 secs
    }
    ATHandler::cmd_start(cmd);
}

We currently plan to override the cmd_start method. A timer _wakeup_timer is started and flag _idle_mode_status is set as soon as idle mode is enabled. The working of above code is as follow:

• If idle mode is not enabled, we simply "pass through" the AT commands.

• If idle mode is enabled but there is serial activity within last 5 seconds, we keep resetting the timer.

• If idle mode is enabled and there is serial inactivity in last 5 seconds, we wake up the modem and then the AT commands are sent as usual.

The wake-up mechanism is to send a character, wait a few milliseconds, send AT command and test response to make sure modem is awake.

Please correct me if i am wrong but I think this is the least intrusive way of doing it. We have handled redundancy and robustness as well. At bottom level, it is just one extra character and few ms delay once every 5 seconds if and only if there is an inactivity.

Could the implementation be similar to ATHandler:::_at_send_delay? That way the time-to-wakeup (6s) and wakeup-delay (1ms) would be given in ATHandler constructor, with default values 0.

Actually the 6s is the time-to-goto-sleep, i.e. if there is no communication between modem and hostMCU in past 6 seconds, the modem goes to sleep.

[AriParkkila]

@wajahat-ublox yes, it looks like a good idea to make cmd_start virtual for now and it can be generalized later if needed.

Thanks for sharing also the code, here are a couple of suggestions.

ATHandler::cmd_start/stop writes also CR, so you are actually writing "A" as wake up string. If that CR is redundant you could send just one byte with ATHandler::write_bytes without start/stop.

It could be also good to change fill_buffer as protected and call that before trying to read with resp_start. The reason I'm not recommending resp here is that it has a bit too much 'semantics' because you should also handle set_at_timeout and clear_error. Also fill_buffer uses poll so it's much more responsive as there is no need to use wait.

The code could then look something like (not tested):
while(time_out--) { ATHandler::write_bytes(buf, 1); if (fill_buffer(50)) { ATHandler::resp_start(); ATHandler::resp_stop(); } }

[wajahat-abbas]

@AriParkkila
Thank you for your suggestions, I will incorporate them when we push the code for this feature.

[0xc0170]

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 2707
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7564/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

[mbed-ci]

Exporter Build : FAILURE

Build number : 2336
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7564/

[mbed-ci]

Test : SUCCESS

Build number : 2438
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7564/2438

[0xc0170]

/morph export-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 2347
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7564/

[jarvte]

You should also change destructor as virtual

[wajahat-abbas]

@jarvte
Thanks for pointing it out. Shall i rebase before pushing the commit?

[jarvte]

@wajahat-ublox no props, you should not need to rebase.

[cmonr]

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 2718
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7564/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 2353
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7564/

[mbed-ci]

Test : SUCCESS

Build number : 2450
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7564/2450

[cmonr]

/morph mbed2-build

[wajahat-abbas]

@AriParkkila
Regarding your suggestion of using fill_buffer, resp_start already calls fill_buffer so i am a little confused. Secondly fill_buffer takes a boolean wait_for_timeout as argument so we again end up waiting?