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

Tolerate some non-utf8 data when detecting boot mode over serial #573

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

kcking
Copy link
Contributor

@kcking kcking commented Feb 3, 2024

My ESP32-WROOM is sending some non-utf8 data just after reset, causing boot mode detection to fail.

This commit uses String::from_utf8_lossy to ignore these bytes.

Tested on macOS, boards: ESP32-WROOM-32, ESP32-WROOM-32D, ESP32-C3-MINI-1, ESP32-C3-12F.

My ESP32-WROOM is sending some non-utf8 data just after reset, causing boot
mode detection to fail.

This commit uses `String::from_utf8_lossy` to ignore these bytes.

Tested on macOS, boards: ESP32-WROOM-32, ESP32-WROOM-32D, ESP32-C3-MINI-1,
ESP32-C3-12F.
Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

Hello! Mind testing what's the behavior with esptool?
My reasoning behind not using _lossy was that it can lead to "unhelpful" error messages, using branch in the #570 scenario, results in:

[2024-02-05T10:08:48Z INFO ] Serial port: '/dev/ttyUSB0'
[2024-02-05T10:08:48Z INFO ] Connecting...
Connect attempt
[2024-02-05T10:08:48Z INFO ] Using flash stub
Error: espflash::timeout

  × Error while connecting to device
  ╰─▶ Timeout while running command

This scenario is not common I'd say, but yet the error message is pretty confusing to the user.

@SergioGasquez SergioGasquez mentioned this pull request Feb 5, 2024
Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution!

@SergioGasquez SergioGasquez merged commit 1cde4fc into esp-rs:main Feb 5, 2024
20 checks passed
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.

2 participants