Fix #3805 #3806
Fix #3805 #3806
Conversation
|
TODO:
|
| }); | ||
| T::regs().cr1().modify(|w| w.set_eocie(true)); | ||
|
|
||
| let mut started = false; |
There was a problem hiding this comment.
I think this shouldn't be needed, it should be OK to start the conversion before registering the waker. Even if the conversion finishes "too soon" the task will see EOC is set and return Ready.
Are you sure these changes are needed to make it work? The "enable interrupt" you added in new is not enough?
There was a problem hiding this comment.
The calling convention is that the operation should be apply when the first time poll is called.
There was a problem hiding this comment.
According to https://doc.rust-lang.org/std/future/trait.Future.html#tymethod.poll
Futures alone are inert; they must be actively polled to make progress, meaning that each time the current task is woken up, it should actively re-poll pending futures that it still has an interest in.
We should do nothing before the Future is polled. (even though the outer asynchronous function will generate such Future, let the internal poll_fn follow this is definitely better)
There was a problem hiding this comment.
Also noticed that
Note that on multiple calls to poll, only the Waker from the Context passed to the most recent call should be scheduled to receive a wakeup.
I will fix it.
There was a problem hiding this comment.
The calling convention is that the operation should be apply when the first time poll is called.
convert is already an async fn. All the code in an async fn is already executed only after polling the future for the first time (the future autogenerated by the compiler for async fn convert()) The poll_fn future is just an "inner" future that the user never sees.
There was a problem hiding this comment.
You are right. But it's nice to make internal Future follow the convention. And I can't see disadvantage there.
There was a problem hiding this comment.
You are right. But it's nice to make internal Future follow the convention. And I can't see disadvantage there.
It makes the code use a bit more flash, it makes it slightly slower, and it is inconsistent with the rest of embassy-stm32.
And I think the change also prevents duplicated trigger the conversion.
it doesn't. code inside an async fn runs only once, the compiler tracks the state of it.
We must register the waker before enabling the EOC interrupt and starting convert