Add async API for CAN#585

Thanks for the PR! We had a brief discussion about it in the weekly meeting today. Having the trait in the asynch module is fine. I think the only remaining question is whether to keep the try_x() methods, move to to another trait, or delete them entirely.

It looks like either the crate's MSRV (and related CI tests) will need updating, or the trait needs to be gated, to get CI passing, too.

@liamkinne

liamkinne commented Apr 4, 2024

Copy link

Copy Markdown

Contributor Author

@adamgreig thanks for the update.

I'm still strongly for keeping the try methods. The use case is that when libraries (bxcan, fdcan, etc) implement these traits they can only implement one trait at a time so they will have a blocking client and an async client. If you're using the async client but need to do something in a non-async context (think RTIC hardware task) you need something to fallback to that doesn't require .await.

It is very similar to the ideas around nb but more like "hey this method will work first time most of the time, unless your buffer is full which you can chose how to handle".

@reneherrero reneherrero mentioned this pull request

Apr 30, 2024

Split embedded-can? #599

Closed

@liamkinne

liamkinne commented Jun 27, 2024

Copy link

Copy Markdown

Contributor Author

@adamgreig in the interest of getting this through so it can start being used downstream, I've removed the try methods.

@adamgreig

adamgreig commented Jun 30, 2024

Copy link

Copy Markdown

Member

Thanks for the prompt and the update. I'll add this to the agenda for discussion in the weekly meeting this Tues.

Dirbaio

Dirbaio reviewed

Jul 2, 2024

View reviewed changes

embedded-can/src/lib.rs Outdated

// We don't immediately remove them to not immediately break older nightlies.

// When all features are stable, we'll remove them.

#![cfg_attr(nightly, allow(stable_features, unknown_lints))]

#![cfg_attr(nightly, feature(async_fn_in_trait, impl_trait_projections))]

@Dirbaio Dirbaio Jul 2, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

these cfg_attrs do nothing without this in build.rs.

These were for compat with older nightlies before AFIT was stable. Now that AFIT has been stable for a few rust versions since 1.75, i'd just not put them, and support stable 1.75+ only.

@liamkinne liamkinne force-pushed the feature/embedded-can-async branch from b8f8ec1 to 37f4be6 Compare

July 4, 2024 10:54

@elpiel

elpiel commented Dec 6, 2024

Copy link

Copy Markdown

Are there any blockers for this PR?

MabezDev

MabezDev previously approved these changes

Feb 7, 2025

View reviewed changes

@MabezDev MabezDev left a comment

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

Based on https://github.com/rust-embedded/embedded-hal/blob/master/docs/how-to-add-a-new-trait.md we should encourage some experimentation here.

IMO with CI fixed, we should merge this it looks like a good first run to me and time will tell if it's suitable in the long term.

@MabezDev MabezDev requested a review from a team

February 7, 2025 12:20

ryankurte

ryankurte reviewed

Feb 10, 2025

View reviewed changes

embedded-can/src/asynch.rs Outdated

/// Puts a frame in the transmit buffer.

/// Awaits until space is available in the transmit buffer.

async fn transmit(&mut self, frame: &Self::Frame) -> Result<(), Self::Error>;

@ryankurte ryankurte Feb 10, 2025

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

i think these should be in the form

fn transmit(&mut self, frame: &Self::Frame) -> Future<Output=Result<(), Self::Error>>;

to avoid needing #![allow(async_fn_in_trait)] (which iirc will be deprecated in the future). you can still implement using async fn in either case.

@robamu

robamu commented Apr 9, 2025 •

edited

Loading

Copy link

Copy Markdown

Contributor

What is the state of this PR? async fn in traits should now work without any additional attributes, right?

robamu

robamu reviewed

Apr 9, 2025

View reviewed changes

embedded-can/src/lib.rs Outdated

#![warn(missing_docs)]

#![no_std]

#![allow(async_fn_in_trait)]

@robamu robamu Apr 9, 2025

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

is this still required on stable Rust?

@elpiel elpiel Aug 7, 2025

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

It is not, however, an appropriate MSRV should be used in Cargo.toml for this.

@liamkinne liamkinne Aug 8, 2025

Copy link

Copy Markdown

Contributor Author

Choose a reason for hiding this comment

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

It is required for the MSRV listed in the Cargo.toml (1.81). Do you want me to keep it or bump the MSRV?

@liamkinne liamkinne dismissed MabezDev’s stale review via

94b2c2d

August 8, 2025 09:16

@liamkinne liamkinne force-pushed the feature/embedded-can-async branch from 37f4be6 to 94b2c2d Compare

August 8, 2025 09:16

@liamkinne

liamkinne commented Aug 8, 2025

Copy link

Copy Markdown

Contributor Author

Before I go any further on this, do we want a single trait to encapsulate transmit and receive or would it be better split into a Tx and Rx trait?

@swobbee-dev

swobbee-dev commented Aug 9, 2025

Copy link

Copy Markdown

I would like to chime in as I have been using this branch for a project for some time now, I actually did implement the split method in my fork because I needed exactly that: https://github.com/swobbee-dev/embedded-hal/tree/feature/embedded-can-async

Therefore I would be greatly in favour of splitting 👍

@pd0wm pd0wm mentioned this pull request

Nov 4, 2025

Use embedded_can traits & types I-CAN-hack/automotive#95

Open

liamkinne added 6 commits

January 24, 2026 20:20

@liamkinne


 add async API

3e70dd2

@liamkinne


 add changelog entry

9ae2e6e

@liamkinne


 fix ci warning

22725ff

@liamkinne


 remove try_ methods

a5c3429

@liamkinne


 Revert "fix ci warning"

a226a0c

This reverts commit 81bc367.

@liamkinne


 split into separate tx/rx traits

68d91c8

@liamkinne liamkinne force-pushed the feature/embedded-can-async branch from 5d35622 to 68d91c8 Compare

January 24, 2026 10:20

@liamkinne

liamkinne commented Jan 24, 2026

Copy link

Copy Markdown

Contributor Author

@adamgreig I've rebased this so it is ready for review again. Notable changes since last time is the trait is now split into tx and rx.

eldruin

eldruin reviewed

Jan 27, 2026

View reviewed changes

@eldruin eldruin left a comment

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

Thank you for this!
Would it be possible to remove the async_fn_in_trait feature if rewriting the signatures like this like stated above?

fn transmit(&mut self, frame: &Self::Frame) -> Future<Output=Result<(), Self::Error>>;

Then we do not need to touch the MSRV. What version would not need it?

@liamkinne


 use impl future

afe2251

@liamkinne

liamkinne commented Jan 27, 2026

Copy link

Copy Markdown

Contributor Author

@eldruin no worries. Have updated the function signatures and left the MSRV as is.

eldruin

eldruin reviewed

Jan 28, 2026

View reviewed changes

@eldruin eldruin left a comment

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

This looks good to me, thank you!
Do you have any implementations of the trait somewhere?

@mickeprag

mickeprag commented Jan 28, 2026

Copy link

Copy Markdown

Do you have any implementations of the trait somewhere?

I am currently waiting for this PR before using it here:
ch32-rs/ch32-hal#146

@liamkinne

liamkinne commented Jan 28, 2026

Copy link

Copy Markdown

Contributor Author

Looking at the embassy fdcan implementation, I realised there's something missing from the transmit signature. It needs to return Ok(Option<Frame>) in the case that the transmit mailboxes are full. This is the existing API with the non-blocking traits.

@liamkinne


 return pending frames with lower priority when the mailbox is full

3c0895c

Labels

None yet

11 participants

@liamkinne @adamgreig @elpiel @robamu @swobbee-dev @mickeprag @eldruin @ryankurte @Dirbaio @burrbull @MabezDev

Conversation

@liamkinne liamkinne commented Apr 2, 2024

Uh oh!

@burrbull burrbull Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

@liamkinne liamkinne Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

adamgreig commented Apr 2, 2024

Uh oh!

liamkinne commented Apr 4, 2024

Uh oh!

liamkinne commented Jun 27, 2024

Uh oh!

adamgreig commented Jun 30, 2024

Uh oh!

@Dirbaio Dirbaio Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

elpiel commented Dec 6, 2024

Uh oh!

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

Uh oh!

@ryankurte ryankurte Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

robamu commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@robamu robamu Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@elpiel elpiel Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

@liamkinne liamkinne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

liamkinne commented Aug 8, 2025

Uh oh!

swobbee-dev commented Aug 9, 2025

Uh oh!

liamkinne commented Jan 24, 2026

Uh oh!

@eldruin eldruin left a comment

Choose a reason for hiding this comment

Uh oh!

liamkinne commented Jan 27, 2026

Uh oh!

@eldruin eldruin left a comment

Choose a reason for hiding this comment

Uh oh!

mickeprag commented Jan 28, 2026

Uh oh!

liamkinne commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

robamu commented Apr 9, 2025 •

edited

Loading