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

uart: fix wdt input overrun #4536

Merged
merged 3 commits into from
Mar 21, 2018
Merged

uart: fix wdt input overrun #4536

merged 3 commits into from
Mar 21, 2018

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Mar 20, 2018

WDT is triggerred when uart is flooded but not read (quickly enough) by user.
Here's a fix with a message on debug console (generally same uart though).

MCVE:

#define PRINT_INTV_MS 5000

unsigned long next;

void setup() {
  Serial.setRxBufferSize(8);
  Serial.begin(115200);
  Serial.setDebugOutput(true);
  Serial.println("please type in chars");
  next = millis() + PRINT_INTV_MS;
}

void loop() {
  static int last = 0;
  int avail = Serial.available();
  if (last != avail)
  {
    last = avail;
    Serial.printf("available: %i\n", avail);
  }

  if (millis() > next)
  {
    Serial.print("reading serial: ");
    while (Serial.available())
      Serial.print((char)Serial.read());
    Serial.println();
    next += PRINT_INTV_MS;
  }
}

A choice has to be made when receive buffer is full:
discard oldest data, or newest data ?
Current master tries to discard newest data (with the wdt bug)
This PR has the two (fixed) ways, I personally prefer discarding the oldest
(with this scheme, I may not be the last leaving among you :)

newest discarded (input is '1234567890'):

00:42:35.122 -> SDK:2.2.1(cfd48f3)/Core:2.4.1-32-g546c8b7/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1)
00:42:35.122 -> please type in chars
00:42:38.464 -> available: 1
00:42:38.465 -> available: 4
00:42:38.465 -> available: 5
00:42:38.497 -> available: 6
00:42:38.497 -> available: 7
00:42:38.497 -> available: 8
00:42:38.497 -> available: 9
00:42:38.497 -> available: 10
00:42:38.497 -> available: 11
00:42:38.497 -> uart input full!
00:42:38.497 -> available: 7
00:42:40.139 -> reading serial: 1234567
00:42:40.139 -> available: 0

oldest discarded (input is '1234567890'):

00:49:12.791 -> SDK:2.2.1(cfd48f3)/Core:2.4.1-32-g546c8b7/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1)
00:49:12.794 -> please type in chars
00:49:15.531 -> available: 1
00:49:15.532 -> available: 4
00:49:15.532 -> available: 5
00:49:15.532 -> available: 6
00:49:15.533 -> available: 7
00:49:15.533 -> available: 8
00:49:15.534 -> available: 9
00:49:15.534 -> available: 10
00:49:15.534 -> available: 11
00:49:15.535 -> uart input full!
00:49:15.535 -> available: 7
00:49:17.796 -> reading serial: 567890
00:49:17.796 -> 
00:49:17.796 -> available: 0
00:49:22.770 -> reading serial: 
00:49:27.779 -> reading serial: 
00:49:32.791 -> reading serial: 

@@ -45,6 +45,7 @@
#include "esp8266_peri.h"
#include "user_interface.h"

uint8_t uart_overrun = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it would be better to put this flag inside the uart struct. Given that the uart struct is opaque, also add an api to check/clear it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@s0170071
Copy link

the example I provided works now, still I get

 ets Jan  8 2013,rst cause:4, boot mode:(3,7)

wdt reset
load 0x4010f000, len 1384, room 16 
tail 8
chksum 0x2d
csum 0x2d
v54cc4634
~ld
⸮U

when I run ESPEasy with serial data incoming and core >2.3.0


// a choice has to be made here,
// do we discard newest or oldest data?
#if 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with either option here.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

I'm ok with either action to take

@devyte
Copy link
Collaborator

devyte commented Mar 20, 2018

I prefer discard newest data, because it keeps the data stream contiguous, but I'm ok with either option.

@devyte
Copy link
Collaborator

devyte commented Mar 21, 2018

HardwareSerial needs a method to expose uart_has_overrun to the user

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 21, 2018

I will intentionally leave the old/new discard #if option choice for further reference.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 21, 2018

@s0170071 thanks for your feedback!
Then further investigations will be needed on ESPeasy side

@s0170071
Copy link

@d-a-v The WD is triggered in WiFi.softAP().

@d-a-v d-a-v merged commit 6580bf3 into esp8266:master Mar 21, 2018
@d-a-v d-a-v deleted the overrun branch March 21, 2018 12:28
@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 21, 2018

@s0170071 please open a new issue with MCVE

bryceschober pushed a commit to bryceschober/Arduino that referenced this pull request Apr 5, 2018
* uart: fix wdt on uart input overrun, expose in HardwareSerial::hasOverrun()

(cherry picked from commit 6580bf3)
@d-a-v d-a-v added this to the 2.4.2 milestone Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants