Skip to content

Conversation

@domodwyer
Copy link

This helps catch accidental misuse of the event loop subscription API, wherein a subscription is created and then immediately removed due to the returned handle not being retained.

It causes a compiler warning to be emitted that looks like this:

warning: unused `EspSubscription` that must be used
  --> src/wifi.rs:56:9
   |
56 | /         event_loop
57 | |             .subscribe::<WifiEvent, _>({
...  |
90 | |             })
91 | |             .expect("failed to subscribe to event bus");
   | |_______________________________________________________^
   |
   = note: Event subscription is unregistered when handle is dropped
   = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
   |
56 |         let _ = event_loop
   |         +++++++

Submission Checklist 📝

  • [N/A] I have updated existing examples or added new ones (if applicable).
  • I have used cargo fmt command to ensure that all changed code is formatted correctly.
  • I have used cargo clippy command to ensure that all changed code passes latest Clippy nightly lints.
  • My changes were added to the CHANGELOG.md in the proper section.

This helps catch accidental misuse of the event loop subscription API,
wherein a subscription is created and then immediately removed due to
the returned handle not being retained.
src/eventloop.rs Outdated
}
}

#[must_use = "Event loop is deleted when handle is dropped"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder why you need that one given that it is an internal type? Just for the esp-idf-svc code itself, or am I missing something?

Copy link
Author

Choose a reason for hiding this comment

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

I was just about to open the PR with the EspSubscription annoation and I thought "I'll quickly check if there's any other Drop impls" and so stuck one on this - didn't even notice it wasn't pub.

I've pushed 717c0fb to remove this one 👍

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