Skip to content

Conversation

c1728p9
Copy link
Contributor

@c1728p9 c1728p9 commented Sep 7, 2017

Set receive timeout to 500ms to prevent dropped data. Update the call to parser.recv to pass NULL rather than a dummy value so only out of band data is processed. This allows parser.recv to return immediately if there is no out of band data.

This patch also updates the ATParser library to bring in updated out of band data processing support.

This PR depends on ARMmbed/ATParser#14

Set receive timeout to 500ms to prevent dropped data. Update the call
in ESP8266::recv to call parser.process_oob rather than parser.recv
since this code is only used to trigger the processing of out-of-band
data.
@c1728p9 c1728p9 force-pushed the fix_dropped_data_from_timeout branch from f35f00a to 8f101bb Compare September 7, 2017 20:36
Copy link
Contributor

@geky geky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

#endif
#ifndef ESP8266_RECV_TIMEOUT
#define ESP8266_RECV_TIMEOUT 0
#define ESP8266_RECV_TIMEOUT 500
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some quick math, 5 symbols (+IPD:) / 115200 baud = 0.043 ms.

500ms > 0.043ms so this looks good 👍

@geky
Copy link
Contributor

geky commented Sep 7, 2017

@marcuschangarm, @bentcooke, this may be a fix for the corruption issue you guys were seeing.

@marcuschangarm
Copy link
Contributor

Awesome! Can someone try the https://github.com/ARMmbed/mbed-stress-test please? I don't have access to an ESP module! 😄

@bentcooke
Copy link

Yes, I will try it with both the CC update and the stress test... though it may be tomorrow before I can get to it. I am having to setup a new laptop right now(old one died!). : (

@c1728p9
Copy link
Contributor Author

c1728p9 commented Sep 7, 2017

Below is my output when I run mbed test -t GCC_ARM -m K64F -n tests-stress-*.

+--------------+---------------+---------------------------+---------------+--------+--------+--------+--------------------+
| target       | platform_name | test suite                | test case     | passed | failed | result | elapsed_time (sec) |
+--------------+---------------+---------------------------+---------------+--------+--------+--------+--------------------+
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer     1  | 1      | 0      | OK     | 42.0               |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer   128  | 1      | 0      | OK     | 35.82              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer   256  | 1      | 0      | OK     | 35.41              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer   512  | 1      | 0      | OK     | 35.44              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer  1k    | 1      | 0      | OK     | 35.4               |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer  2k    | 1      | 0      | OK     | 31.19              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer  4k    | 1      | 0      | OK     | 29.13              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer  8k    | 1      | 0      | OK     | 41.58              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer 16k    | 1      | 0      | OK     | 29.23              |
| K64F-GCC_ARM | K64F          | tests-stress-filesystem   | Buffer 32k    | 1      | 0      | OK     | 28.36              |
| K64F-GCC_ARM | K64F          | tests-stress-multitasking | Buffer  4k    | 1      | 0      | OK     | 114.56             |
| K64F-GCC_ARM | K64F          | tests-stress-multitasking | Buffer  8k    | 1      | 0      | OK     | 93.44              |
| K64F-GCC_ARM | K64F          | tests-stress-multitasking | Buffer 16k    | 1      | 0      | OK     | 84.64              |
| K64F-GCC_ARM | K64F          | tests-stress-multitasking | Buffer 32k    | 1      | 0      | OK     | 81.61              |
| K64F-GCC_ARM | K64F          | tests-stress-multitasking | Setup network | 1      | 0      | OK     | 5.33               |
| K64F-GCC_ARM | K64F          | tests-stress-network      | Download  4k  | 1      | 0      | OK     | 127.0              |
| K64F-GCC_ARM | K64F          | tests-stress-network      | Download  8k  | 1      | 0      | OK     | 94.69              |
| K64F-GCC_ARM | K64F          | tests-stress-network      | Download 16k  | 1      | 0      | OK     | 84.51              |
| K64F-GCC_ARM | K64F          | tests-stress-network      | Download 32k  | 1      | 0      | OK     | 80.0               |
| K64F-GCC_ARM | K64F          | tests-stress-network      | Setup network | 1      | 0      | OK     | 7.33               |
+--------------+---------------+---------------------------+---------------+--------+--------+--------+--------------------+
mbedgt: test case results: 20 OK

@marcuschangarm
Copy link
Contributor

Beautiful! Thank you Russ! 😄

@JanneKiiskila
Copy link

Merge it in, ASAP. :-)

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bulislaw bulislaw merged commit fdb44a4 into master Sep 8, 2017
@c1728p9 c1728p9 deleted the fix_dropped_data_from_timeout branch September 8, 2017 15:19
@geky
Copy link
Contributor

geky commented Sep 8, 2017

I see we need a needs dependency label...

@bentcooke
Copy link

I have been able to test this with a CC FW update config that was previously failing every time and it appears to have fixed the image corruption issue. Nice work Russ!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants