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

Added docstrings for wifi module #262

Merged
merged 2 commits into from
Jun 3, 2023
Merged

Added docstrings for wifi module #262

merged 2 commits into from
Jun 3, 2023

Conversation

IvanSanchez
Copy link
Contributor

I recently became slightly frustrated with some of the documentation of the Wifi module. I noted that WifiDriver just calls functions from esp_idf_sys (which do have more detailed notes about their behaviour and preconditions), and also noted that EspWifi, BlockingWifi and AsyncWifi are just wrappers, adding links to the functions being wrapped.

Signed-off-by: Iván Sánchez Ortega <ivan@sanchezortega.es>
@Vollbrecht
Copy link
Collaborator

❤️ thanks for helping out documenting stuff !

src/wifi.rs Outdated Show resolved Hide resolved
src/wifi.rs Outdated Show resolved Hide resolved
@@ -1625,6 +1664,8 @@ fn matches_wifi_event(event: &WifiEvent) -> bool {
)
}

/// Wraps a [`WifiDriver`] or [`EspWifi`], and offers strictly synchronous (blocking)
/// function calls for their functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might want to add an example here in that wrapping is possible both by moving the WifiDriver / EspWifi instance into BlockingWifi, as well as by mutable borrowing it (i.e. &mut wifi_driver), where the latter allows one to re-use the driver elsewhere once BlockingWifi is dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alas, I wasn't even aware that BlockingWifi was wrapping a EspWifi instance until I read the source code - I don't have a use case where I'd reuse the driver by dropping BlockingWifi, much less I feel confident to write a reliable example.

I'd rather leave this for a future improvement.

src/wifi.rs Outdated Show resolved Hide resolved
src/wifi.rs Outdated Show resolved Hide resolved
src/wifi.rs Outdated Show resolved Hide resolved
Signed-off-by: Iván Sánchez Ortega <ivan@sanchezortega.es>
@ivmarkov ivmarkov merged commit b97f7a8 into esp-rs:master Jun 3, 2023
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.

3 participants