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

Fix crash via cmd "Status 0" #3313

Closed
wants to merge 1 commit into from
Closed

Conversation

ckesc
Copy link

@ckesc ckesc commented Jul 23, 2018

If you execute Status 0 you'll get a crash Soft WDT reset.
It can be caused by Home Assistant monitoring too

This is because of too long operation in PublishStatus funciton (>50ms)
Now we use yield() to let Wifi module process its data.

See more at
http://arduino-esp8266.readthedocs.io/en/latest/reference.html?highlight=yield#timing-and-delays

If you execute "Status 0" you'll get
a crash "Soft WDT reset".
This is because of too long operation in "PublishStatus" funciton (>50ms)
Now we use yield() to let Wifi module process its data.

See more at
http://arduino-esp8266.readthedocs.io/en/latest/reference.html?highlight=yield#timing-and-delays
@ascillato
Copy link
Contributor

ascillato commented Jul 23, 2018

Hi,

Sorry, I'm not having that crash.

Do you have that issue in the default version?

@Erelen-Laiquendi
Copy link

Erelen-Laiquendi commented Jul 23, 2018

Same crash with Tasmota 6.1.1c (sonoff-classic.bin) on Sonoff POW2 caused by TasmoAdmin.

@ascillato
Copy link
Contributor

ascillato commented Jul 23, 2018

Have you checked the #3289 ?

There were some binaries with problems due to travis issue.

Please, use the recently published binaries by Theo, those are ok.

Or you can build it by yourself also.

@ckesc
Copy link
Author

ckesc commented Jul 24, 2018

Yes, I checked.
It seems that my issue is reproducable on Sonoff POW2.

Bacause:

  • for this device default Baudrate is set to 4800.
  • this device have much information to print in response to Status 0

All of this causes long execution of MqttPublishPrefixTopic_P

Printing one status doesn't take too much time. But if execute all statuses at once - you get long operation, that takes more than 50 ms.
That's why the software watchdog resets the device.

@arendst
Copy link
Owner

arendst commented Jul 24, 2018

Cannot reproduce.

Pls provide output of status 0 as I suspect you are using syslog...

arendst added a commit that referenced this pull request Jul 24, 2018
Fix possible WDT due to long MQTT publish handling (#3313)
@arendst
Copy link
Owner

arendst commented Jul 24, 2018

Pls try the change just released.

@ckesc
Copy link
Author

ckesc commented Jul 24, 2018

Partially fixed.

There is no crashes now.
But it seems that the device is still doing too much work when generating Status 0 report.
If I open Tasma Admin (which polls Status 0 every 5 seconds) and start ping, I got gaps: 2-3 packet loss (see https://gist.github.com/ckesc/7c970b1c57b53f8d7f29070af545ffd4)

And if I change Baudrate to 115200, I get good ping: no packet loss, 1-26 ms

I looks like this bug appeared at 6.x.x versions.
Maybe you changed something, that made message sending heavier?
Is there is a way to profile perfomance?

@ascillato
Copy link
Contributor

Is there is a way to profile perfomance?

Yes, you can recompile with Debug ON for CPU usage. Please, see the file xdrv_99_debug.ino and enable the key //#define USE_DEBUG_DRIVER

@ascillato
Copy link
Contributor

Please, provide also your STATUS 0 output and your findings. Thanks

@arendst
Copy link
Owner

arendst commented Jul 25, 2018

A status 0 every 5 seconds is a disaster waiting to happen...

I know TasmoAdmin uses the HTTP interface to get information but as status 0 results in many MQTT messages (and probably syslog and serial if enables) too it's a major impact on Tasmota performance. If you enable serial AND a low baudrate exceptions are expected as the output just cannot be delivered in time.

You'll have to decide if you want serial output anyway once your device is in production.

As the main problem is solved by introducing a yield I hereby close this PR.

@arendst arendst closed this Jul 25, 2018
curzon01 pushed a commit to curzon01/Tasmota that referenced this pull request Sep 7, 2018
Fix possible WDT due to long MQTT publish handling (arendst#3313)
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.

4 participants