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

Ensure AT interface is responsive #1886

Merged
merged 3 commits into from Aug 20, 2019

Conversation

@zfields
Copy link
Contributor

commented Aug 15, 2019

Long timeouts from Publish Vitals use of COPS command when AT interface is unresponsive
Story details: https://app.clubhouse.io/particle/story/37231


  • [enhancement] ensures AT interface is responsive #1886

@zfields zfields added this to the 1.3.1-rc.1 milestone Aug 15, 2019

@zfields zfields requested a review from technobly Aug 15, 2019

@zfields zfields self-assigned this Aug 15, 2019

@zfields

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

TIMEOUT NOTES:
line numbers are approximate

USOCL_UDP_TIMEOUT (10s)
mdm_hal.cpp|2200:

  • Behind check @ 2191
    mdm_hal.cpp|2352:
  • Behind check @ 2350

USOCL_TCP_TIMEOUT (120s)
mdm_hal.cpp|2200:

  • Behind check @ 2191
    mdm_hal.cpp|2352:
  • Behind check @ 2350

USOCO_TIMEOUT (120s)
mdm_hal.cpp|2301:

  • Behind check @ 2295

USOWR_TIMEOUT (120s)
mdm_hal.cpp|2421:

  • Behind check @ 2416
    mdm_hal.cpp|2425:
  • Behind check @ 2422
    mdm_hal.cpp|2495:
  • Behind check @ 2489

USORD_TIMEOUT (10s)
mdm_hal.cpp|2447:

  • Behind check @ ???

USOST_TIMEOUT (40s)
mdm_hal.cpp|2531:

  • Behind check @ 2525
    mdm_hal.cpp|2535:
  • Behind check @ 2532
    mdm_hal.cpp|2601:
  • Behind check @ ???

USORF_TIMEOUT (10s)
mdm_hal.cpp|2557:

  • Behind check @ ???

COPS_TIMEOUT (180s)
mdm_hal.cpp|906:

  • Behind check @ 901
    mdm_hal.cpp|1253:
  • Behind check @ 1249
    mdm_hal.cpp|1277:
  • Behind check @ 1273
    mdm_hal.cpp|1339:
  • Behind check @ 1333
    mdm_hal.cpp|1343:
  • Behind check @ 1339
    mdm_hal.cpp|1388:
  • Behind check @ ???
    mdm_hal.cpp|1398:
  • Behind check @ 1388
    mdm_hal.cpp|1446:
  • Behind check @ ???
    mdm_hal.cpp|1452:
  • Behind check @ 1446
    mdm_hal.cpp|2094:
  • Behind check @ 2092

CPWROFF_TIMEOUT (40s)
mdm_hal.cpp|1056:

  • Behind check @ 1011

UPSDA_TIMEOUT (180s)
mdm_hal.cpp|1848:

  • Behind check @ 1841
    mdm_hal.cpp|1901:
  • Behind check @ 1897
    mdm_hal.cpp|2021:
  • Behind check @ ???
    mdm_hal.cpp|2064:
  • Behind check @ ???

CFUN_TIMEOUT (180s)
mdm_hal.cpp|980:

  • Behind check @ ???
hal/src/electron/modem/mdm_hal.cpp Show resolved Hide resolved
hal/src/electron/modem/mdm_hal.cpp Outdated Show resolved Hide resolved
hal/src/electron/modem/mdm_hal.cpp Outdated Show resolved Hide resolved
hal/src/electron/modem/mdm_hal.cpp Outdated Show resolved Hide resolved
hal/src/electron/modem/mdm_hal.cpp Outdated Show resolved Hide resolved
@technobly

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

The job exceeded the maximum time limit for jobs, and has been terminated.

boron xsom build timed out after 50 minutes for some reason... I restarted it

@technobly
Copy link
Member

left a comment

Looks good, I would like to test this before we merge it. I'll create a build and run it overnight.

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

While testing I noticed there are no similar AT/OK checks on socketRecv and socketRecvFrom like there are for socketSend and socketSendTo

Could we add the USORD_TIMEOUT define here with the && _atOk() check

if (ISSOCKET(socket) && (_sockets[socket].pending == 0)) {
sendFormated("AT+USORD=%d,0\r\n", _sockets[socket].handle); // TCP
waitFinalResp(NULL, NULL, 10*1000);

And also here could we add the USORF_TIMEOUT define here with the && _atOk() check

if (ISSOCKET(socket) && (_sockets[socket].pending == 0)) {
sendFormated("AT+USORF=%d,0\r\n", _sockets[socket].handle); // UDP
waitFinalResp(NULL, NULL, 10*1000);

I started searching for other hard coded long timeouts and found these. Could we add definitions for these and _atOk() checks? SORRY for the last minute Friday ask... please wait until Monday!

sendFormated("AT+UBANDSEL=%s\r\n", bands_to_set);
if (RESP_OK == waitFinalResp(NULL,NULL,40000)) {

sendFormated("AT+CGATT=1\r\n");
if (RESP_OK != waitFinalResp(NULL,NULL,3*60*1000))

sendFormated("AT+CGATT=0\r\n");
if (RESP_OK != waitFinalResp(NULL,NULL,3*60*1000)) {

sendFormated("AT+UDNSRN=0,\"%s\"\r\n", host);
if (RESP_OK != waitFinalResp(_cbUDNSRN, &ip, 30*1000)) {

sendFormated("AT+CMGS=\"%s\"\r\n",num);
if (RESP_PROMPT == waitFinalResp(NULL,NULL,150*1000)) {

@zfields zfields requested a review from technobly Aug 19, 2019

zfields added 3 commits Aug 15, 2019
Ensure AT interface is responsive
- Tests to ensure AT interface will respond to "AT"
- Only affects Gen2 devices

@technobly technobly force-pushed the ch37231/at-ifc branch from 9829758 to f52573f Aug 20, 2019

@technobly technobly merged commit cdb6beb into develop Aug 20, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@technobly technobly deleted the ch37231/at-ifc branch Aug 20, 2019

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