coralreef has quit [Quit: Do not go gentle into that goodnight.]
notgull has quit [Ping timeout: 256 seconds]
emerent has quit [Ping timeout: 260 seconds]
emerent has joined #rust-embedded
M762spr[m] has joined #rust-embedded
<M762spr[m]>
Sorry, not exactly rust related (although my tool is being written in rust 😅) but do any of the chip manufacturers publish machine readable chip description files? Like a SVD but describing timers and pin mapping... like what is used in cubeMX. Surely there are config files for that sort of thing somewhere? I'd rather not have to manually enter it from the datasheet for each chip I use 🫠
<M762spr[m]>
I think I may have found some of what I am looking for although these xml files are not very intuitive... Definitely going to need to parse these into something actually useful.
<dirbaio[m]>
<M762spr[m]> "I think I may have found some of..." <- > <@762spr:matrix.org> I think I may have found some of what I am looking for although these xml files are not very intuitive... Definitely going to need to parse these into something actually useful.... (full message at <https://catircservices.org/_matrix/media/v3/download/catircservices.org/OLgjRXqlBzteViYRBgxTwFFL>)
<M9names[m]>
<thejpster[m]> "uh, I can't immediately put my..." <- were you using an esp32c? the esp-rs folks don't use riscv-rt, they roll their own rt impl.
<vollbrecht[m]>
potato: check out the rust std-training for esp32. You can check out the rgb-led lib inside the training. It is a simple and good working solution to drive different kinds of smart-leds llike the ws2812
<Ralph[m]>
<mameluc[m]> "> <@rursprung:matrix.org> what's..." <- > <@mameluc:matrix.org> enum Errors are nice for debugging and you can choose what to do with different kinds of errors. A balance has to be found on what is too much information. If something fails in rtic init it is probably fine with unwrap as you can't... (full message at <https://catircservices.org/_matrix/media/v3/download/catircservices.org/rzsDHrRECBFZBxDDlmCWnRKF>)
<vollbrecht[m]>
it uses the esp rmt hardware device for the timed pulses, if you want to use the code just copy the rgb-led src/lib into your project its pretty simple, no external crate needed
<Ralph[m]>
<JamesMunns[m]> "In general, I usually tend to..." <- how do you do that in a driver crate where you don't know the peripheral error type? what kind of information do you keep/extract? do you have an example?
<Ralph[m]>
JamesMunns[m]: ok, so this is essentially the same as proposed in the PR which i linked (except that you have a simple `I2c::Error` while that PR has 5 different errors from 2 e-h types to deal with).
<Ralph[m]>
you mentioned that you `impl From`, but i didn't see that in your example?
<Ralph[m]>
so would you - unlike @mameluc - recommend to propagate all errors (as proposed in the PR) or only a subset of them / an abstraction over them (simple "hardware error" with little more details) in the case of my lib?
<JamesMunns[m]>
Hmm, yeah I'd have to look why I didn't impl From there.
<JamesMunns[m]>
I think the answer is probably "it depends", giving downstream users the option of using that context is good form, but I'd guess pragmatically most aren't going to do MUCH with it, esp for things like gpio pins
<JamesMunns[m]>
So yeah, I usually add one variant to my outer error type that is "the generic peripheral had an error". I'm unsure how pleasant this would be if you had 5 different generic tied to 2 different traits.
<dirbaio[m]>
if you have multiple errors you can't impl From for all of them because the compiler can't prove they don't overlap
<GrantM11235[m]>
In my experience it is rare actually handle OutputPin errors, since they are so uncommon
<GrantM11235[m]>
No, I can see two options to fix it
<dirbaio[m]>
if a CS pin starts erroring
<dirbaio[m]>
you don't know anymore what state it's in
<GrantM11235[m]>
one: only allow infallible cs pins (or unwrap in the impl)
<GrantM11235[m]>
two: keep track of whether the bus is "poisoned" (has a cs pin of unknown state). return an error for all transactions until it is fixed by re-de-asserting the offending cs pin
<dirbaio[m]>
how common is it to want to recover after a GPIO error?
<dirbaio[m]>
this seems to me a bit theoretical, usually you'd handle these by crashing and rebooting
<GrantM11235[m]>
Isn't that the whole point of fallible gpio? To handle errors without just crashing?
<dirbaio[m]>
yes, but maybe you want to do some cleanup or log which gpio failed before crashing?
<dirbaio[m]>
dunno
<dirbaio[m]>
I feel option 1 (restrict to infallible gpio) is a bit unnecessarily restrictive.
<GrantM11235[m]>
And that still doesn't solve the problem in my example. Thread 2 can try to do a transaction before thread 1 has a chance to crash
<GrantM11235[m]>
dirbaio[m]: You can do that in your infallible/panicking gpio impl
<dirbaio[m]>
and option 2 would have some cost. I think it's a bit complex to implement, you need the "poisoned" flag but also to store which device CS is stuck so you clear only after that. which seems a bit tricky because you can create/destroy devices any time.
<dirbaio[m]>
and the cost would affect even if CS is infallible, I don't think Rust can optimize out the flag because it's in the struct. (we could copypaste everything for fallible and infallible, but i'd rather not)
<dirbaio[m]>
GrantM11235[m]: with the poisoning, thread 1 CS failure would've caused thread 2 to fail anyway.
<GrantM11235[m]>
I think you would need a shared flag to store that some cs on the bus is poisoned, plus a flag for each device to say that my cs is poisoned
<dirbaio[m]>
GrantM11235[m]: then if you destroy a poisoned device, the bus gets poisoned forever.
<GrantM11235[m]>
dirbaio[m]: Yes, which is better than having two cs pins active at once
<dirbaio[m]>
yeah... hmm
<dirbaio[m]>
I still think adding the poisoning infra just for this extreme edge case is a bit heavy-handed
<dirbaio[m]>
it's a very rare edge case
<dirbaio[m]>
- fallible GPIO (rare)
<dirbaio[m]>
- AND the gpio actually fails (rare)
<dirbaio[m]>
- AND the user wants to somehow recover from it instead of crashing and rebooting.
notgull has quit [Ping timeout: 255 seconds]
<GrantM11235[m]>
There is definitely a cost to handling fallible cs pins, but if we only want to support infallible or panicking pins, we should be honest about it
<dirbaio[m]>
we should document what the impl does in case of errors, so the user is aware of it
<GrantM11235[m]>
Why not just require infallible pins?
<dirbaio[m]>
becasue there's no reason to
<dirbaio[m]>
the current impl does support fallible pins just fine, just with this caveat
<dirbaio[m]>
we should document it so the user knows that if they want to actually handle the error they need to take some hardware-specific corrective action
<dirbaio[m]>
if they want to just panic instead of handlign it, the current impl works just fine
<dirbaio[m]>
s/handlign/handling/
<GrantM11235[m]>
dirbaio[m]: No it doesn't there is still the race condition
<dirbaio[m]>
if the hardware is broken, there's tons of other race conditions possible
<dirbaio[m]>
what if your SPI returns an error, but failed to stop the underlying transfer and keeps clocking out bytes? another thread might assert another CS and accidentally send those bytes to the other device
<dirbaio[m]>
I think it's perfectly reasonable to document the error handling behavior and declare it "working as intended"
<dirbaio[m]>
it doesn't have to be all or nothing (either forbid infallible pins, or support them adding cost for everyone for every single edge case)
<dirbaio[m]>
we can support fallible pins in a "best-effort" way and document we do so, and then users with the more extreme requirements can write their own SpiDevice impls
sjm42[m] has quit [Quit: Idle timeout reached: 172800s]
<GrantM11235[m]>
We would also need to document that device drivers are never allowed to throw away spi errors
<GrantM11235[m]>
Because something like let val = self.spi.some_transaction().unwrap_or_default(); could be hidden in a generic device driver, which could break the whole bus without the user knowing about it
<dirbaio[m]>
why would a driver do that?
<dirbaio[m]>
silently ignoring all failures like that is bad practice
<dirbaio[m]>
like, I can see a driver checking if the failure is an I2C NAK and then ignoring / retrying / delaying, but not ignoring ALL failures
notgull has joined #rust-embedded
<GrantM11235[m]>
It may be bad practice, but it is very unexpected for it to break the entire bus imo
<GrantM11235[m]>
There may be other cases where a driver throws away an spi error, for example if it needs to report another error instead
mabez[m] has quit [Quit: Idle timeout reached: 172800s]
<GrantM11235[m]>
Like let res = spi_transaction(); cleanup()?; return res
<dirbaio[m]>
this is the edge case of the edge case
<dirbaio[m]>
🤣
<Ralph[m]>
<GrantM11235[m]> "Like let res = spi_transaction(..." <- well, clearly Rust is just missing `try { .. } catch (Exception ex) { .. } finally { .. }`!
<dirbaio[m]>
gotta catch'em all
<dirbaio[m]>
well
<vollbrecht[m]>
and finally kill yourself ☠️
<dirbaio[m]>
jokes aside GrantM11235 if you want, could you open an issue and we can discuss it next meeting?
<dirbaio[m]>
despite my pushback it is indeed a legitimate issue
<GrantM11235[m]>
sure 👍️
<vollbrecht[m]>
* kill yourself (mcu) ☠️
<dirbaio[m]>
I sort of lean towards not doing bus poisoning because of my personal bias for simplicity
<dirbaio[m]>
but it should get some discussion, if other HAL team menbers feel poisoning (or forbidding infallible, or another solution) is worth it then we should do it
<dirbaio[m]>
thanks for bringing it up :)
<Ralph[m]>
sorry for that joke before, just couldn't resist ^^
<Ralph[m]>
but i do wonder: is there a clean way to the equivalent of `finally` in rust with `Result`-handling? that'd handle the example above
<GrantM11235[m]>
By the way, there are other benefits to option one (infallible/panicking cs only), it allows us to get rid of the error enum
<dirbaio[m]>
that's true 👀
<GrantM11235[m]>
I'm kinda leaning toward option one. Users still have the option of implementing option two on their own
<dirbaio[m]>
yeah
<dirbaio[m]>
hmmhmhmhm
<dav1d>
Ralph[m], the finally equivalent would be something that runs something on Drop
<GrantM11235[m]>
we could have both options, but I don't really think it's worth it
<Ralph[m]>
dav1d: that's the equivalent of a destructor in the likes of C++/Java.
<Ralph[m]>
but you usually don't really have any `struct` going out of scope when you have `foo()?;` returning with an `Err`, so on what would you `impl Drop` here?
<dav1d>
Ralph[m], `let _cleanup = Cleanup::new(); foo()?; ....` or with a separate scope `let x = { let _cleanup = Cleanup::new(); foo()? } `
<dav1d>
pretty much what mutex guards, etc. do
<dav1d>
can be cheeky and make cleanup something generic that calls a closure on drop
<GrantM11235[m]>
The point of that example was that the cleanup is also fallible. You can't return an error from Cleanup::drop
<dav1d>
GrantM11235[m], you cant return in the catch block and then also return from the finally block