Skip to content
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.

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

Add into_async for ParlIO #2461

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Add into_async for ParlIO #2461

merged 1 commit into from
Nov 4, 2024

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Nov 4, 2024

Forgot to update parl_io in #2430. Still not sure what to do about TxOnly/RxOnly.

Copy link
Collaborator

@Dominaezzz Dominaezzz left a comment

Choose a reason for hiding this comment

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

Still not sure what to do about TxOnly/RxOnly.

At some point we should support converting each half of the DMA channel to and from blocking mode (on chips that an support it), which should solve this I think.

@bugadani
Copy link
Contributor Author

bugadani commented Nov 4, 2024

We can do some runtime checks on chips that don't support it (unless the presence of the async handler poses a problem in blocking mode).

@Dominaezzz
Copy link
Collaborator

unless the presence of the async handler poses a problem in blocking mode

It does, users could end up overriding the async handler, rendering the async driver broken.
Unless we implement some handler sharing logic like GPIO does but I don't like this direction.

@bugadani
Copy link
Contributor Author

bugadani commented Nov 4, 2024

It does, users could end up overriding the async handler, rendering the async driver broken.

That's what I mean by runtime checks, I'd just add a panic to prevent this.

Unless we implement some handler sharing logic like GPIO does but I don't like this direction.

Me neither.

@Dominaezzz
Copy link
Collaborator

That's what I mean by runtime checks, I'd just add a panic to prevent this.

Oh right, got it. I suppose there's a risk that the async handler would clear the interrupt bits that the blocking driver is polling for maybe.

Also, if it's gonna be a panic, might as well use the type system to prevent it. i.e. use cfg to remove the APIs on chips that don't support it.

@bugadani
Copy link
Contributor Author

bugadani commented Nov 4, 2024

Also, if it's gonna be a panic, might as well use the type system to prevent it. i.e. use cfg to remove the APIs on chips that don't support it.

"If it's gonna be a panic, don't use a panic" is a weird thing to say.

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

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

Thanks!

@JurajSadel JurajSadel added this pull request to the merge queue Nov 4, 2024
Merged via the queue into esp-rs:main with commit ed7960c Nov 4, 2024
28 checks passed
@bugadani bugadani deleted the parlio branch November 4, 2024 16:26
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.

3 participants