Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Board will not enter Deep Sleep after communications with Cell Module #11413

Closed
JRubis opened this issue Sep 4, 2019 · 19 comments · Fixed by #11623

Comments

@JRubis
Copy link

@JRubis JRubis commented Sep 4, 2019

Description

Hi,

My board is an STM32l4-discovery with Quectel BG96 connect via the ST-MOD+ connector.
I am using mbed-OS and the commit is : 279f4cd from 7/10/2019. I can't use an official release because the additions that support cellular through the ST-MOD+ connection will not be available until v5.14.

If I run the following code, my board behaves well and results in an average MCU current draw of around 15uA.

int main() {

   mbed_file_handle(STDIN_FILENO)->enable_input(false);

   while (1) {
       led = !led;
       ThisThread::sleep_for(60000);

       mbed_stats_cpu_t stats;
       mbed_stats_cpu_get(&stats);
       printf("Uptime: %llu ", stats.uptime / 1000);
       printf("Sleep time: %llu ", stats.sleep_time / 1000);
       printf("Deep Sleep: %llu\n", stats.deep_sleep_time / 1000);
}
...
}

If however I access the BG96 to read its cellular information and then enter this "sleep loop" the current consumption is around 7mA. Obviously something is blocking deep sleep and I am not sure what it is. Is it the UART that is used for communication between the MCU and BG96? Is it the AT Service? Something else...

I have tested in both AT and PPP modes.

I attempted to enable "MBED_SLEEP_TRACING_ENABLED" to see what is blocking sleep but that results in the application not getting to main(). I think its possibly running very slowly as opposed to crashing. I have bumped the "rtos.idle-thread-stack-size" to 1024 and that doesn't help.

If I place a break point in the function sleep_manager_lock_deep_sleep_internal() I can see repeated calls from static void LPTIM1_IRQHandler(void).

Ultimately what I am attempting to achieve is to wake up at a periodic interval connect to a server via cell, send some data, disconnect and then reenter deep sleep. What network or device API's calls are required to shutdown the network / device to achieve this?

Issue request type

[x] Question
[ ] Enhancement
[ ] Bug
@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 5, 2019

OK I found the main issue. Of course an instance of SerialBase is created to support communications between the MCU and cellular module as part of CellularDevice::get_default_instance(). By default, serial input is enabled. This blocks deep sleep. It appears that none of the methods of CellularDevice disable input on this serial port which would allow the MCU to deep sleep. You would think that methods such as hard_power_off() or soft_power_off() would do this? Am I missing something?

@JRubis JRubis closed this Sep 5, 2019
@JRubis JRubis reopened this Sep 5, 2019
@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 5, 2019

After disabling the serial input as part of soft_power_off(), I am still seeing an elevated current draw of about 300uA. Not sure where that is coming from at this point. Still looking.

@0xc0170

This comment has been minimized.

Copy link
Member

@0xc0170 0xc0170 commented Sep 6, 2019

cc @ARMmbed/team-st-mcd if you got any hints where it can come from

@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 6, 2019

OK. So the additional ~300uA was coming from a floating pin. Pin #1 on the STMOD+ connector which is connected to UART1_CTS on the STM32 doesn't have a external pull up or pull down resistor. This allows the pin to float and increases the current consumption.

I could have added an external pull down resistor, but I elected to use an internal one.

I am not sure of the correct way to do this in MBED since the pin is technically not used as a normal DI.

I created a DI mapped to the same pin as the UART's CTS pin

DigitalIn cts(MBED_CONF_STMOD_CELLULAR_CTS);

And then set its pull mode:

cts.mode(PullDown);

This seems a bit hokey to me but it appears to work OK.

So in summary I believe there are a two possible bugs here:

  1. The serial port is being left enabled by the cellular framework not allowing the MCU to enter deep sleep. This may not be specific to the STM32L496 disco and BG96.

  2. A pull down needs to be enabled on MBED_CONF_STMOD_CELLULAR_CTS

@LMESTM

This comment has been minimized.

Copy link
Contributor

@LMESTM LMESTM commented Sep 6, 2019

@JRubis thanks for this.
About 2
I'm note sure what's the best place to apply the PullDown (and how to do so).
Shall we do it in here https://github.com/ARMmbed/mbed-os/blob/master/components/cellular/COMPONENT_STMOD_CELLULAR/STModCellular.cpp
But then how can we apply a pullDown to a Serial pin (without using the DigitalIn as you did)

About 1. someone from cellular team should help on this ?

@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 6, 2019

@LMESTM No problem. I would say it would make sense to place it in that file. As it is specific to the STMOD+ Cellular Interface. Sorry I can't be of more help on how. I am rather new to mbed and just finding my way around. I will say that my "hack" isn't the way.

I need to look at PPP mode next week. Using that interface still results in a average current draw in sleep of ~400uA as opposed to ~20uA in "AT Mode."

@kjbracey-arm

This comment has been minimized.

Copy link
Contributor

@kjbracey-arm kjbracey-arm commented Sep 9, 2019

There is no C++ Mbed OS API, for applying pulls to non-GPIO pins. (Whether this is even possible will be platform-specific - the pull circuitry could be either side of the pinmux).

Calling pin_mode(pin, pulltype) after activating the non-GPIO peripheral should work, if a platform can do it. You have to do it after because pinmap_pinout will calls pin_mode(pin, PullNone) when the non-GPIO peripheral is activated.

@AriParkkila

This comment has been minimized.

Copy link
Contributor

@AriParkkila AriParkkila commented Sep 17, 2019

It appears that none of the methods of CellularDevice disable input on this serial port which would allow the MCU to deep sleep. You would think that methods such as hard_power_off() or soft_power_off() would do this? Am I missing something?

At the moment, hard/soft_power_off does not release UART but you need to implement that yourself, e.g.:

nsapi_error_t MY_QUECTEL_BG96::soft_power_on()
{
    _fh->enable_input(true);
    _fh->enable_output(true);
    ...
}

nsapi_error_t MY_QUECTEL_BG96::soft_power_off()
{
    ...
    if (_at->get_last_error() == NSAPI_ERROR_OK) {
        _fh->enable_input(false);
        _fh->enable_output(false);
    }
    return _at->unlock_return_error();}

If you want to keep BG96 in network (e.g. PSM) then you should probably do it in your application (instead of soft_power_off()) e.g.:

    CellularDevice::get_default_instance()->get_file_handle().enable_input(false);
    CellularDevice::get_default_instance()->get_file_handle().enable_output(false);
    mbed_file_handle(STDIN_FILENO)->enable_input(false);
    mbed_file_handle(STDIN_FILENO)->enable_output(false);
@0xc0170

This comment has been minimized.

Copy link
Member

@0xc0170 0xc0170 commented Sep 17, 2019

At the moment, hard/soft_power_off does not release UART but you need to implement that yourself, e.g.:

@AriParkkila Should it or what is on master is good as it is?

@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 17, 2019

@AriParkkila yes this is in fact what I had to implement. Is this something that was overlooked and should be added? In my opinion the application shouldn't need to directly manage a resource that it didn't directly create.

As for PSM, I believe once the BG96 is issued a software power down command, PSM becomes non functional anyway.

@AriParkkila

This comment has been minimized.

Copy link
Contributor

@AriParkkila AriParkkila commented Sep 18, 2019

In my opinion the application shouldn't need to directly manage a resource that it didn't directly create.

@JRubis I guess you are referring here to get_default_instance(), but if you want to handle the life-cycle of CellularDevice properly, you probably should create it with new QUECTEL_BC95(FileHandle *fh), in which case you'd also create fh yourself.

@0xc0170 I think master is now good as it is, at least until we get some more design ready.

@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 18, 2019

Hi @AriParkkila what I am trying to say is the application really doesn't "know" that an instance of UARTserial was created to support the cellular interface nor should it need to concern itself with that. Any operations on the serial port should be done by the Cellular Device, the "owner." Having the application code control or directly mange resources it didn't explicitly create could lead to problems. Is there a reason why you think the application should do this? Maybe I am missing something here. Thanks.

@AriParkkila

This comment has been minimized.

Copy link
Contributor

@AriParkkila AriParkkila commented Sep 19, 2019

@JRubis You need to pass UARTSerial to BG96 driver when it's created, e.g.:

#include "QUECTEL_BG96.h"
void main() {
    UARTSerial serial(MBED_CONF_QUECTEL_BG96_TX, MBED_CONF_QUECTEL_BG96_RX, MBED_CONF_QUECTEL_BG96_BAUDRATE);
    CellularDevice *dev = new QUECTEL_BG96(&serial);
    CellularContext *ctx = dev.create_context();
    ctx->connect();
    // when needed application could now disable serial to enable deep-sleep
    serial.enable_input(false);
    serial.enable_output(false);
}
@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Sep 19, 2019

@AriParkkila There isn't a need for the application to do this directly.

I simply added calls to disable the serial port in & out in STModCellular::soft_power_off () and made sure to enable them in STModCellular::soft_power_on(). They may also need to be added in the case of hard power on/off as well. My board does not support those.

For instance:

nsapi_error_t STModCellular::soft_power_off()
{
	print_function("\n\nsoft_power_off()");
    _at->cmd_start("AT+QPOWD");
    _at->cmd_stop();
     wait_ms(1000);
    // should wait for POWERED DOWN with a time out up to 65 second according to the manual.
    // we cannot afford such a long wait though.
	
 	_fh->enable_output(false);
 	_fh->enable_input(false);		
	
    return STMOD_CELLULAR_MODEM::soft_power_off();
}

These changes could have alternatively been made to the class STModCellular is derived from in this case QUECTEL_BG96

@AriParkkila

This comment has been minimized.

Copy link
Contributor

@AriParkkila AriParkkila commented Sep 20, 2019

@JRubis Thanks for sharing. Your code should work in many cases. If Mbed OS CPU needs to deep-sleep while the modem is powered on (e.g. in PSM) or the same UART is shared with another device, UART can be disabled on the application as shown above.

LPTIM1_IRQHandler could be ticking due to missing some TICKLESS configuration in json?

@JRubis JRubis closed this Sep 20, 2019
@jeromecoutant

This comment has been minimized.

Copy link
Contributor

@jeromecoutant jeromecoutant commented Oct 2, 2019

Hi @JRubis
In your final version, did you apply some patch in STM or cellular side ?
Maybe you could push a pull request ?
Thx

@JRubis

This comment has been minimized.

Copy link
Author

@JRubis JRubis commented Oct 3, 2019

Hi @jeromecoutant, There were two issues here. One was with the missing pull down on CTS and the other was with the serial port receive enable. I am not sure which one your a referencing? Based on @AriParkkila's response it seems ARM wants the application to handle the management of the serial port. They feel this is a better approach. So I didn't submit a pull request for that change.

For the pull down it wasn't clear to me if @LMESTM was going to take care of this or the expectation was that I would submit a pull request.

@AriParkkila

This comment has been minimized.

Copy link
Contributor

@AriParkkila AriParkkila commented Oct 3, 2019

@jeromecoutant @JRubis I need to correct that a bit. Please see my comments about how to work with the current implementation until we get some more design ready.

@loverdeg-ep

This comment has been minimized.

Copy link
Contributor

@loverdeg-ep loverdeg-ep commented Oct 10, 2019

@trowbridgec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.