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.

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

Jump to bottom

Uart: problems after added check of err_wr_mask in config #1580

Open
Tracked by #2495
maxwen opened this issue May 23, 2024 · 7 comments · May be fixed by #2756
Open
Tracked by #2495

Uart: problems after added check of err_wr_mask in config #1580

maxwen opened this issue May 23, 2024 · 7 comments · May be fixed by #2756
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. peripheral:uart UART peripheral

Comments

@maxwen
Copy link
Contributor

maxwen commented May 23, 2024 •
edited
Loading

I have written an esp32 mbus scanner for reading the smartmeter of my energy provider.
It uses an mbus scanner attached to uart (from MikroElektronika).

That worked fine before the following was added to uart.rs

...
// Setting err_wr_mask stops uart from storing data when data is wrong according
// to reference manual
T::register_block()
.conf0()
.modify(|_, w| w.err_wr_mask().set_bit());
...

And the subsequent check for events in read_async
...
let mut events = ....
| RxEvent::RxFrameError
| RxEvent::RxGlitchDetected
| RxEvent::RxParityError;
...

I can bring it back working as before when disabling setting err_wr_mask and disabling that
those three events return an Err.

The read loop is based on rx_timeout to sync the packets which are sent periodically
from the smartmeter. Its a very slow connection (2400 baud)

Now I am unsure why this creates those "errors". Is this just "normal" for those
kind of connections? If yes - would it make sense to add some kind of config
to disable those "forced" checks "on demand"?

So I guess it comes down to what exactly means "data is wrong according to reference manual"
@maxwen maxwen changed the title Uart: problems aftter added check of Uart: problems after added check of err_wr_mask in config May 23, 2024
@jessebraham jessebraham added the peripheral:uart UART peripheral label May 23, 2024
@dignifiedquire
Copy link

having the same issue, would be great if this could be a config option

@MabezDev
Copy link
Member

We'll get to this eventually, but a PR would be most welcome :).

@clabbenius
Copy link

Do you have an example code snippet where you use read_async uart? And what are you experiencing?

@dignifiedquire
Copy link

I don’t have a link right now, but what I am doing is implementing LIN over uart and need to read the initial break signal, which does not translate cleanly to a uart signal.

@MabezDev MabezDev mentioned this issue Nov 22, 2024
Open
42 tasks
@zpg6
Copy link

zpg6 commented Dec 9, 2024

I don’t have a link right now, but what I am doing is implementing LIN over uart and need to read the initial break signal, which does not translate cleanly to a uart signal.

+1, came here for the same reason

@zpg6 zpg6 linked a pull request Dec 13, 2024 that will close this issue
Draft
@bugadani
Copy link
Contributor

bugadani commented Dec 13, 2024 •
edited
Loading

I suspect the two parts of this issue are independent:

  • err_wr_mask makes the peripheral essentially forget data that has partiy/framing/other errors, but otherise I think it should continue receiving data. I'm not exactly sure disabling this is useful, you'd have no information about the correctness of what you receive - but it's a hardware feature and we should expose it.
  • The async API is indeed stopping at faults, and a somewhat arbitrary list of those (cc Not all UART hardware interrupts are supported/handled #2643). We'll have to provide some API where users can wait for bytes, while controlling which error conditions they are interested in.

@zpg6
Copy link

zpg6 commented Dec 13, 2024

I suspect the two parts of this issue are independent:

  • err_wr_mask makes the peripheral essentially forget data that has partiy/framing/other errors, but otherise I think it should continue receiving data. I'm not exactly sure disabling this is useful, you'd have no information about the correctness of what you receive - but it's a hardware feature and we should expose it.

  • The async API is indeed stopping at faults, and a somewhat arbitrary list of those (cc Not all UART hardware interrupts are supported/handled #2643). We'll have to provide some API where users can wait for bytes, while controlling which error conditions they are interested in.

Ok, agreed. I'm most interested in a reasonable method to get my async reads to work. Best if such an error could be ignored on just one byte (like a break), and other bytes that follow can be read successfully.

@tom-borcin tom-borcin added 1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. and removed 1.0-blocker labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. peripheral:uart UART peripheral
Projects
esp-rs
Status: Todo
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(uart): config option to disable the rx input checks zpg6/esp-hal
8 participants
@dignifiedquire @bugadani @jessebraham @maxwen @MabezDev @zpg6 @tom-borcin @clabbenius