Conversation
Ssid::as_str truncates at the first invalid UTF-8 byte and the Wi-Fi standard allows any 32 bytes, so a scanned non-UTF-8 network name has no public way to be read back out as bytes for storage. `Ssid` already implements `TryFrom<&[u8]>` publicly, which covers rebuilding an SSID from stored bytes; only the accessor to get the bytes out was missing.
fbartho
requested review from
JurajSadel,
MabezDev and
bjoernQ
as code owners
September 12, 2026 22:42
bjoernQ
approved these changes
Sep 17, 2026
bjoernQ
left a comment
bjoernQ
left a comment
Contributor
There was a problem hiding this comment.
I'd say the hint about as_str would belong to that method and we don't really need a justification for a method to be present.
However we will need another round of API docs review before esp-radio 1.0 proper anyways
MabezDev
requested changes
Sep 17, 2026
MabezDev
left a comment
MabezDev
left a comment
Member
There was a problem hiding this comment.
Please mark this API as unstable before we merge this, using the instability attribute.
Contributor
|
If you touch this, it makes sense to move the |
Review feedback on esp-rs#6322: the new accessor is gated behind the `unstable` feature, and the note about lossy UTF-8 handling moves to `as_str`, the method whose behavior it describes.
Author
|
Addressed in d965e59. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ssid::as_strtruncates at the first invalid UTF-8 byte, and the Wi-Fi standard allows an SSID to be any 32 bytes, not necessarily valid UTF-8. A scanned network name that isn't UTF-8 has no public way to be read back out as bytes for storage.Ssidalready implementsTryFrom<&[u8]>publicly (esp-radio/src/wifi/mod.rs:843-854), which covers rebuilding anSsidfrom stored bytes. The only missing half is the accessor to get the bytes back out, so this PR makesas_bytespublic (esp-radio/src/wifi/mod.rs:785-793), matching the visibility ofas_str,len, andis_emptyon the same type, with a doc comment.This is a visibility widening on a pre-1.0 crate, so it's additive only; nothing existing changes behavior.
I used AI assistance (Claude) to prepare this branch; I reviewed, tested, and understand the change before opening this PR, per the project's AI tool use policy.
Testing
cargo xtask fmt,cargo xtask lint esp-radio esp32s3, andcargo xtask documentation --packages esp-radiopass clean. No test added:esp-radiohas no host-test path and this file's doctests areno_run, and the change only widens the visibility of an existing method.Changelog
Validated with
echo "<body>" | cargo xtask check-pr-changelogagainst this exact body (including the prose around it): "PR description changelog format is valid."