esp_restart() in refactor parser#204
esp_restart() in refactor parser#204blattm wants to merge 4 commits intoCarlosDerSeher:refactor_parserfrom
Conversation
blattm
changed the title
agreed
I'd prefer not to reboot the ESP if possible. That's what I meant by handling the error gracefully. Keep the system running but log the error. |
|
If we prioritize keeping refactoring and behavior changes separate, then we should only merge this after refactor_parser itself was merged. I changed some of the reboots to network resets. But all of the remaining reboots might indicate issues with resources (like out-of-memory situations), unreachable states, issues with the communication to the player, etc. |
luar123
left a comment
luar123
left a comment
There was a problem hiding this comment.
Something is not working properly when the connection is restarted: Sometimes the base message is corrupted, giving a huge size that needs to be handled by unknown message parser. Does not happen every time and can be triggered by restarting the snapserver while playing. With dev branch I could not trigger it.
(Did not try this PR, just refactor_parser branch)
E (35687) CONNECTION_HANDLER: netconn err -15
I (35688) CONNECTION_HANDLER: Wait for network connection
I (35689) CONNECTION_HANDLER: try connecting to static configuration xxx
E (35698) CONNECTION_HANDLER: can't connect to remote xxx, err -14
W (36319) PLAYER: RESYNCING HARD 2: age 19us, latency 7731216287293us, free 197720, largest block 110592, 0, rssi: -54
I (36704) CONNECTION_HANDLER: Wait for network connection
I (36705) CONNECTION_HANDLER: try connecting to static configuration xxx
I (36711) CONNECTION_HANDLER: netconn connected using WIFI_STA_DEF
I (36715) SC: sta mac: xxx
I (36721) SC: netconn sent hello message
I (36726) SC: rx base message type: 354, size: 12976303
I (38320) AUDIO_HAL: Codec mode is 2, Ctrl:0
I (38321) PLAYER: stop player done
| return -1; | ||
| } | ||
| case PARSER_INCOMPLETE: { | ||
| // need more data |
There was a problem hiding this comment.
should we free wire_chnk / decoderChunk / pcmData here?
3 participants
Critical errors now explicitly restart the esp. Previously this was done by bubbling up an error value and returning from the
http_get_task. This also simplified the code a little bit.But there are still some questions:
esp_restart()the "correct" function to call? The doc says it does not restart peripherals. Maybeabort()behaves more similar to the old behavior?EDIT: Maybe we want to keep changes to the behavior and the refactoring separate. Otherwise pinning down where potential issues were introduced might become very hard in the future...