Skip to content

Poll for completion on SPI write. #131

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 28, 2020
Merged

Poll for completion on SPI write. #131

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9044ed1
Poll for completion on SPI write.
newAM Dec 24, 2020
05f3e34
Move the bsy polling to check_send.
newAM Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/spi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ where
nb::Error::Other(Error::ModeFault)
} else if sr.crcerr().bit_is_set() {
nb::Error::Other(Error::Crc)
} else if sr.txe().bit_is_set() {
} else if sr.txe().bit_is_set() && sr.bsy().bit_is_clear() {
return Ok(());
} else {
nb::Error::WouldBlock
Expand Down Expand Up @@ -482,7 +482,7 @@ where
}

// Do one last status register check before continuing
self.check_send().ok();
nb::block!(self.check_send()).ok();
Copy link

Choose a reason for hiding this comment

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

Still .ok() rather than returning the result?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it as ok() because that better preserves the previously existing behavior.

...as for whether or not it should be ok() or ? that is debatable. What do you think?

I think the previous intent was to clear the overrun flag if it was set, but that requires a read to DR followed by a read access to SR to clear.

Copy link

Choose a reason for hiding this comment

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

My understanding is that before it had no tangible effect (since it didn't read from DR) and now it blocks but still does not affect the flags. It seems a bit strange if we were to call this function and identify a failure but not report it -- then again, if it didn't prevent transmission, perhaps it's best to ignore it. It may just depend on the error.

All in all, I don't really have an influencing perspective here since I'll be cherry-picking and tacking on my own changes in my project's fork (I need the non-blocking behavior in some cases). The .ok() pattern to ignore an error seems strange to me since it looks more like a mistake, but if this is the behavior that works, I can't say much against it!

Thanks for fixing this! 😄

Ok(())
}
}
Expand Down Expand Up @@ -526,7 +526,7 @@ where
}

// Do one last status register check before continuing
self.check_send().ok();
nb::block!(self.check_send()).ok();
Ok(())
}
}