Adding sync version of CfuWriter #21
Merged
Conversation
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
magravel
requested review from
RobertZ2011,
felipebalbi,
jerrysxie and
tullom
as code owners
magravel
force-pushed
the
magravel/small_improvements
branch
from
4c977d3 to
5cf94f3
Compare
magravel
force-pushed
the
magravel/small_improvements
branch
2 times, most recently
from
e5fe69b to
93cc60a
Compare
remove send and add noop writer. revmove default impl cargo fmt cargo clippy review fix cargo +nightly fmt fix doc all features cargo fmt line with 100 Noop -> Nop fmt remove send trait, future pr add mut to self in writer trait cargo fmt cargo +nightly fmt try to make the ci fmt works :(
magravel
force-pushed
the
magravel/small_improvements
branch
from
93cc60a to
43eafb0
Compare
RobertZ2011
previously approved these changes
Jun 10, 2025
madeleyneVaca
previously approved these changes
Jun 11, 2025
Contributor
Yes |
felipebalbi
reviewed
Jun 17, 2025
RobertZ2011
approved these changes
Jun 17, 2025
Ctru14
reviewed
Jun 27, 2025
JamesHuard
approved these changes
Jun 27, 2025
Contributor
Author
|
Hi, I looked at the maybe-async-cfg crate. Having a Sync version of CfuWrite is to be used when small packets need to be sent. In that case, the use of a non-async API could result in better performance. In a project containing these use cases, it is expected to use both sync and async versions of the CfuWriter. Adding a feature to the build to have or not the sync version adds lots of complexity and no benefit in this use case. One benefit could be that we do not have to write the trait implementation twice, but since for now it is only four lines, adding a macro to the code and new dependencies seems overkill. |
kurtjd
pushed a commit
to OpenDevicePartnership/embedded-services
that referenced
this pull request
Jul 7, 2025
…ge (#384) There was a breaking change and minor version update (0.2.0) in embedded-cfu via OpenDevicePartnership/embedded-cfu#21. While the necessary changes are integrated, limit version to 0.1.0 for the workspace in cargo.toml
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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.
This PR adds a sync version of
CfuWriterand mergedCfuWriterMutwithCfuWriter.CfuWriteris now namedCfuWriterAsyncand the synchronous version is namedCfuWriterSync. To keep the previous behavior, useCfuWriterAsync.CfuWriterNopto be more explicit about what it does.Issues
Fix #5
Fix #17