Skip to content

Conversation

@okhsunrog
Copy link

@okhsunrog okhsunrog commented May 30, 2025

This PR makes #[bisync] a bit smarter for async code. You can now tell it to automatically add a suffix to your .awaited method calls.

For example, if your underlying library has device.read() and device.read_async(), you can now do this:

#[bisync(async_suffix = "async")]
pub async fn get_some_data(&mut self) -> Result<Data, Error> {
    // Async: becomes self.device.read_async().await?
    // Sync:  becomes self.device.read()?
    let data = self.device.read().await?;
    Ok(data)
}

This helps avoid manual #cfgs for these common async/sync method differences.

How it Works:

  • When you're in an asynchronous module:
    • #[bisync(async_suffix = "your_suffix")] will change method().await to method_your_suffix().await.
    • Just #[bisync] (no suffix argument) on an async fn will still work as before – no magic renaming of .await calls inside.
  • Sync module behavior for #[bisync] is unchanged.

Why is this useful? (Example with device-driver)

The device-driver crate generates register access methods like some_register().read() for synchronous operations and some_register().read_async() for asynchronous ones.

Without this PR, you might write:

#[bisync]
async fn read_status_register(&mut self) -> Result<StatusFieldSet, MyError> {
    #[cfg(feature = "async")]
    let status_data = self.low_level_device.status_reg().read_async().await?;
    #[cfg(feature = "blocking")]
    let status_data = self.low_level_device.status_reg().read()?;
    Ok(status_data)
}

With this PR, it becomes cleaner:

#[bisync(async_suffix = "async")] // Suffix for device-driver's async methods
async fn read_status_register(&mut self) -> Result<StatusFieldSet, MyError> {
    // Automatically calls .read_async().await in async, .read()? in sync
    let status_data = self.low_level_device.status_reg().read().await?;
    Ok(status_data)
}

This significantly reduces boilerplate when wrapping device-driver generated APIs or similar libraries with distinct sync/async method names.

@JM4ier
Copy link
Owner

JM4ier commented May 30, 2025

I'm not sure if this is a good addition to the library, so I have a couple of questions.

Is this useful for anything else than wrapping a pair of functions to have the same name in the current scope? I imagine it gets weird once you have multiple calls in a function body, some of which might need the suffix and some of which might not need it.

Would it not be possible to achieve the same using something like this (roughly the same amount of code):

#[only_sync]
use path::to::Struct::read as Struct_read;

#[only_async]
use path::to::Struct::read_async as Struct_read;

You could even put all of these uses in the top-level modules if the code structure is similar enough to the example:

#[path = "."]
pub mod asynchronous {
    use bisync::asynchronous::*;

    use path::to::somewhere::read as read;

    mod inner;
    pub use inner::*;
}

#[path = "."]
pub mod blocking {
    use bisync::synchronous::*;

    use path::to::somewhere::read_async as read;

    mod inner;
    pub use inner::*;
}

@okhsunrog
Copy link
Author

Well, the primary goal is exactly that, not just a pair, all pairs of functions with symmetric names. It's true, it does get weird if I'd have multible calls in function body, but for this use case it works, all the async calls would be from the same device-driver crate, which all have _async suffix.
doing this

#[only_sync]
use path::to::Struct::read as Struct_read;

#[only_async]
use path::to::Struct::read_async as Struct_read;

is not really possible in my case, as I'm calling the methods on generated code. Higher in my code I have this line:

device_driver::create_device!(device_name: AxpLowLevel, manifest: "device.yaml");

it generated code from a yaml with registers description. The yaml has 2040 lines of code, which generate 18K lines of Rust code with the macro. Total number of fields in registers is about 160 and almost all of them have read / read_async, write / write_async, modify / modify_async.

so the first problem is: there are too many fields with these methods to write use path::to::somewhere::read_async as read for each of them, the second problem is that the code I'm referring to is generated with the macro at compile time

so I tried doing my best to prevent code dublication

@okhsunrog
Copy link
Author

@JM4ier here's how I use it https://github.com/okhsunrog/axp192-dd

@okhsunrog
Copy link
Author

Any suggestions how to improve it?

@okhsunrog
Copy link
Author

another suggestion:

#[bisync]
async fn read_status_register(&mut self) -> Result<StatusFieldSet, MyError> {
    // Automatically calls .read_async().await in async, .read()? in sync
    let status_data = bisync_suffix!("_async", self.low_level_device.status_reg().read().await?);
    // Don't change suffix here
    Timer::after(Duration::from_secs(1)).await;
    Ok(status_data)
}

@okhsunrog
Copy link
Author

I already have a draft of this approach that works for me, I can crete a second PR, if you like this approach more

@okhsunrog
Copy link
Author

I released bisync_suffix_macro crate so closing these 2

@okhsunrog okhsunrog closed this Jun 1, 2025
@JM4ier
Copy link
Owner

JM4ier commented Jun 1, 2025

I only got time to look at it now, sorry for the delay.

The main objective of me writing another library to write async-generic code is to be more minimal than maybe-async, i.e. to not need to depend on syn parsing and quoting, and to work in the presence of feature unification.

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features.

axp192-dd could probably be made to work to enable both blocking and async at the same time if it follows the example in the documentation (but as it currently stands it doesn't).

Another thing I'd like to note is that bisync_suffix_macro as it's written right now can't be made to work with feature unification because it disables blocking codegen if async is enabled. Also it's dependent on what a library developer calls the feature flags (blocking and async sound reasonable, but that's a thing that's not mandated by bisync)

@JM4ier JM4ier reopened this Jun 1, 2025
@okhsunrog
Copy link
Author

I don't think that enabling both blocking and async support of the driver makes sense. A system that has the axp192 power management IC almost always has only one such IC, and the interface that get's passes to Axp192 when creating one usually implements either I2C from embedded-hal or embeddded-hal-async traits. They could be both implemented, but that's a rare case and I don't see any practical sense in using both async and blocking calls as the same time.
So I see it just a alternative to having to support 2 separate drivers, one blocking and one async. Almost all drivers in embedded rust are either blocking or async, in my implementation the user decides which one to use.
Talking about feature unification, it's not always feasible, for example in embassy-stm32, esp-hal, and a lot of hardware-related packages the feature flag is used to select the chip variant that the project is running on.
About using syn and quote, I though that using these is okay, because a lot of my deps use them anyway so they are compiled anyway.
About the bisync_suffix_macro, I should probably allow using of both blocking and async at the same time.
So what do you think about axp192-dd do you think I should support both blocking and async at the same time? If yes, how do you suggest doing it without significant code duplication?

@okhsunrog
Copy link
Author

I can do it another way probably, introducing a single feature flag is_sync, the same way maybe_async does it, this should be enough. Because I can't have both async and blocking enabled, and I can't have neither or them enabled, in this case just using one feature flag should be enough

@JM4ier
Copy link
Owner

JM4ier commented Jun 2, 2025

do you think I should support both blocking and async at the same time?

For a direct user of the library it might not make a lot of sense but it does have effects on downstream libraries that might use axp192-dd. I.e. if library A depends on the blocking version of axp192-dd, library B depends on the async version of apx192-dd, and library C depends on A and B it will only work if apx192-dd supports compiling blocking and async at the same time.

If yes, how do you suggest doing it without significant code duplication?

the apa102-spi library does it very easily like this:

#[path = "."]
mod asynchronous {
    use bisync::asynchronous::*;
    use embedded_hal_async::spi::SpiBus;
    use smart_leds_trait::SmartLedsWriteAsync as SmartLedsWrite;
    mod writer;
    pub use writer::*;
}
pub use asynchronous::Apa102Writer as Apa102WriterAsync;

#[path = "."]
mod blocking {
    use bisync::synchronous::*;
    use embedded_hal::spi::SpiBus;
    use smart_leds_trait::SmartLedsWrite;
    #[allow(clippy::duplicate_mod)]
    mod writer;
    pub use writer::*;
}
pub use blocking::Apa102Writer;

note the duplicate mod writer; to avoid duplicating the module source.

@okhsunrog
Copy link
Author

@JM4ier thank you, I put some time and impoved my axp192-dd crate according to your advice, now I don't have blocking and async features, my driver supports both, similar to apa102-spi. I also found a way to do this without the suffix thing, I just defined these functions: https://github.com/okhsunrog/axp192-dd/blob/main/src/bisync_helpers.rs
I think now this can be closed and maybe I should yank the bisync_suffix_macro crate.

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