DepthDeluxe has joined #rust-embedded
starblue1 has joined #rust-embedded
fabic has joined #rust-embedded
starblue has quit [Ping timeout: 245 seconds]
PyroPeter has quit [Ping timeout: 252 seconds]
PyroPeter has joined #rust-embedded
DepthDeluxe has quit [Ping timeout: 268 seconds]
<re_irc> <> I don't know if y'all have checked out STM32 I2C clock config, but it's remarkably opaque, with ST implying you should use Cube to calculate it
<re_irc> <> I think I have a working setup built with preset speeds extrapolated from the tables ST provides
<re_irc> <> nRF's setup is much easier - set a bit according to a preset speed and... that's it. On the register level
<re_irc> <> It's also notable that ST copy+pasted the RM sections; the I2C periph clock speeds it uses for its tables are the same across MCUs that operate at very diff speeds
dcz has joined #rust-embedded
dcz_ has joined #rust-embedded
dcz has quit [Ping timeout: 268 seconds]
fabic has quit [Ping timeout: 268 seconds]
emerent has quit [Read error: Connection reset by peer]
emerent has joined #rust-embedded
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 260 seconds]
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 268 seconds]
<re_irc> <> Does anyone know of a proc-macro that checks that all enum variants implement a trait?
<re_irc> <> On a different note, is there a modern replacement to ssmarshal that anyone recommends? Ssmarshal has the annoying property that if you don't have enough buffer to deserialize something it panics in debug, rather than what it does in release and returns an `Err`.
<re_irc> <> Seems like might be a good replacement
<re_irc> <> Or maybe not, was not `serde` based
<re_irc> <> Might just be simpler to fork `ssmarshal`, fix it, and release `ssmarshal-fixed` :P
<re_irc> <> korken89: on a tangent: do you know how much overhead ssmarshal has for serializing a plain array of i16?
<re_irc> <> ssmarshal looks a bit abandoned. no commits and that and other bugs for >3 years.
<re_irc> <> Not really
<re_irc> <> It has quite tiny footprint though generally
<re_irc> <> Yeah, it needs some love
<re_irc> <> But it's perfect for really simple stuff, like just stuffing bytes in a radio buffer
<re_irc> <> I should make a comparison on ssmarshal vs postcard for this
<re_irc> <> Also the libs do a lot of hoop jumping
<re_irc> <> E.g. deserializing a `u32` goes through taking the buffer, converting to array (with all the error handling), and then `::from_le_bytes(..)` where the same can be made with an unaligned read (or swap + unaligned read) depending on what one wants
<re_irc> <> ssmarshal is quite nice there though, they chose the lesser evil of bitshifts and or
<re_irc> <> I really need to do perf measurements of these crates
<re_irc> <> I'd really like the tiniest simplest lib with serde support but most are with a lot of bells and whistles
<re_irc> <> (which makes them more complete for general usecases)
<re_irc> <> perf + size
<re_irc> <> Tinyness should not be forgotten :)
<re_irc> <> Hmm though sometimes it seens like using `serde` is quite a negative
<re_irc> <> Instead of doing one size check and then blindly serializing/deserializing, the buffer is checked in every element of serialization/deserialization
<re_irc> <> Seems like ssmarshal should be able to get around it, as the serialize and deserialize functions has known upper bounds on the needed buffer size
<re_irc> <> If buffer size >= known upper bound do blind deserialization
<re_irc> <> Hmm, I should stop rambling and start implementing :P
GenTooMan has quit [Ping timeout: 268 seconds]
<re_irc> <> Right, it works fine with `serde` as well though without all the error checks it seems to inline a lot better
GenTooMan has joined #rust-embedded
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 265 seconds]
fabic has joined #rust-embedded
<re_irc> <> korken89: interested to hear how postcard doesn't fit your use case! Postcard ranks pretty well on both size and speed in the serde benchmarks I've seen:
<re_irc> <> IIRC from my original benchmarking, it was a few cycles slower than ssmarshal, but like... single to double digits, even for reasonably sized data
<re_irc> <> You could try to use that bench from ssmarshal, but it has limitations like no arrays over 255 bytes, or no non-`#[repr(C)]` types.
<re_irc> <> jamesmunns: Nice, thanks for the benchmark!
<re_irc> <> The thing I have the biggest issue with is the buffer checks, over and over and over
<re_irc> <> Deserializing a u32 should not be more than 2 clock with an unaligned read for example
<re_irc> <> But if you have upper bounds on the buffer needed, you should get away with only one check
<re_irc> <> And then blindly do serialization
<re_irc> <> I mean, the problem is for formats that contain (for example) enums or arrays, you may not know the upper bound before hand
<re_irc> <> Then you have enums and such which are sepcial
<re_irc> <> This is the nice property of ssmarshal, it is always known
<re_irc> <> I guess if you knew a GLOBAL upper bound (e.g. Heapless Vec instead of arrays), you might
<re_irc> <> It's simple in that case, don't allow reference types
<re_irc> <> `&[...]`/`&str`
<re_irc> <> I like reference types for zero cost deserialization, btw :D
<re_irc> <> BUT
<re_irc> <> I have thought about having a KnownSize trait
<re_irc> <> to be able to calculate things like buffer size needed for example
<re_irc> <> I could see having a `no_check_deserialize() -> T` where `T: Deserialize + KnownMaxSize`
<re_irc> <> then use all unchecked access after a single check
<re_irc> <> and for example, implementing that for arrays and Heapless structures, but not for slices.
<re_irc> <> you could always have a wrapper type with a declared max size, that would fail ser/de if it was ever greater than reported
<re_irc> <> Makes sense
<re_irc> <> At least from `postcard` as it's designed around allowing reference types :)
<re_irc> <> The next issue I have is using subslices with `from_xx_bytes` for integers instead of an unaligned pointer read
<re_irc> <> Hmm
<re_irc> <> As the subslicing often does not optimize away properly
<re_irc> <> (often in conjunction with the size checks)
<re_irc> <> that's harder because the LINE format of postcard is defined to be LE, you can use it on a BE machine
<re_irc> <> You can check this with the endianness flag
<re_irc> <> unaligned read for same, bitshifts and or for not
<re_irc> <> I'm open to adding cfgs to the deser/ser impl that check this!
<re_irc> <> Ooor
<re_irc> <> I'm not sure if those were stable when I first wrote postcard
<re_irc> <> Ah right, probably not
<re_irc> <> But I have a pretty yolo opinion when it comes to MSRV :)
<re_irc> <> If theres a way to do it now, I'd love to have those
<re_irc> <> The thing is that when I check the disasm, I expect to see mostly reads from memory, but the ser/deser codes are always riddled with branchres
<re_irc> <> In the end I understand why it happens, but it just seem very inefficient to me
<re_irc> <> `#[repr(C, Packed)]` is always the arcane "gotta go fast" option :D
<re_irc> <> you'll pay the cost elsewhere though
<re_irc> <> Yeah
<re_irc> <> I mean I think a lot could be done with just reducing the size checks
<re_irc> <> And not going via subslices
<re_irc> <> (just a hunch)
<re_irc> <> But I'm getting more and more thirst for giving it a proper evaluation
<re_irc> <> Please do! happy to have improvements without safety compromises in postcard, even if it means breaking changes :D
<re_irc> <> Hehe :D
<re_irc> <> The main thing is though it will be a lot more `unsafe` code
<re_irc> <> But hey, our ethernet stacks still run so whatthehell
<re_irc> <> * We have tests that exercise the potentially risky cases
<re_irc> <> * We document the reasons
<re_irc> <> Not terribly frightened by unsafe code, as long as:
<re_irc> <> * We have benches that prove that there is an actual improvement
<re_irc> <> postcard should be a pretty good candidate for miri testing, anyway
<re_irc> <> Nice
<re_irc> <> (since you can do it with no I/O or allocator)
<re_irc> <> bbqueue is like... 100% unsafe anyway :D
<re_irc> <> Hehe
<re_irc> <> As is `heapless` :)
<re_irc> <> but, I also have a fair amount of tests, including Miri tests that get run by CI.
<re_irc> <> whiiiiiich I broke in my last release :D (mostly need to update the test steps because I pulled some features)
<re_irc> <> ahhh
<re_irc> <> I'm remembering wrong
<re_irc> <> I don't have miri tests in CI, but I do have sanitizer tests
<re_irc> <> I did get miri running at one point, and actually found some theoretical UB, but it required some duct tape, and I never cleaned that back up
<re_irc> <> Ah
<re_irc> <> I like serializers and deserializers as often what one want is a mem read/write
<re_irc> <> The question is how to trick the Rust compiler into giving us what we want :D
<re_irc> <> (not included in this is enums and all types derived from them where the discriminant needs a check)
<re_irc> <> (and reference types)
<re_irc> <> Like I said, I think having trait bounds for this would be good, potentially with a completely different serializer impl
<re_irc> <> (since specialization isn't a thing)
<re_irc> <> Yeah
<re_irc> <> I was using ssmarshall as a base as its codebase is tiny
<re_irc> <> honestly, the first draft of postcard was kind of a mashup of ssmarshal and bincode, more or less
<re_irc> <> tbh the only notable change from bincode's wire format is that I specify it's always LE, and I use varints for arrays and enum discriminants
<re_irc> <> Yeah, I have used postcard for a lot
<re_irc> <> But sometimes it feels weird when the serde stuff of a binary is 10k (where a similar in an oooold C project that is super unsafe) is sub 1k
<re_irc> <> I wonder where the tradeoffs are and where we can make the Rust side a lot better
<re_irc> <> It seems like branches and template instantiations play together here
<re_irc> <> I did notice in a recent project that a couple of inlines, and actually NOT doing postcard+cobs in a single pass made things significantly faster
<re_irc> <> Yeah
<re_irc> <> I have seen the same
<re_irc> <> We do it in 2 passes in our stuff
<re_irc> <> Instead of using flavours
<re_irc> <> Yeah, I think the recursive nature of flavors, while convenient, are a little pathological to inline/optimize
<re_irc> <> But when you start adding COBS and stuff a lot can happen when inlining goes happy :)
<re_irc> <> But yeah, I think I really want to see if I can get close to the old C stuff in safe Rust youknow
<re_irc> <> And at least understand why we might no get 100% there
<re_irc> <> > I split out the "postcard" and "encoding" steps, then smothered rlercobs in #[inline(always)], and encoding is now twice as fast
<re_irc> <> This was for ~4k data chunks, so the delta was pretty significant in that project.
<re_irc> <> Cool
<re_irc> <> ah, forgot an important part:
<re_irc> <> > and the total process is 1/3 faster.
<re_irc> <> so total serialization + encode time was 1/3 faster
<re_irc> <> just encoding was twice as fast
<re_irc> <> Not bad
<re_irc> <> ```diff
<re_irc> <> - ticks_encoding: 3159535
<re_irc> <> + ticks_encoding: 2134174
<re_irc> <> (1 uS ticks)
<re_irc> <> or actually, that might be 250ns ticks
<Lumpio-> Oh since we're talking about postcard, how stable/versionable is the protocol?
<re_irc> <> Jebus, 2 seconds for encoding 4k?
<Lumpio-> Does it for instance use Rust enum discriminants which may change?
<re_irc> <> that was 250ns ticks, but that was total accumulated time spent over a second
<re_irc> <> so looooooots of 4k messages
<re_irc> <> Rust discriminants in serde are always specified as the order of definition
<re_irc> <> so, if you reorder your enum, that would be a breaking wire format change
<re_irc> <> Someone wrote up a post recently, that I agree with, is that you should probably have specific `Raw` or `Wire` types that you `Into` into internal data structures
<re_irc> <> Ahhh makes sense
<re_irc> <> I don't always do it, but I agree it's best practice, especially if you care about wire formats and breaking changes
<re_irc> <> (for hobby projects, I just recompile and reflash everything)
<re_irc> <> but postcard is a non-self-describing format, so you always HAVE to have the schema the same on both sides.
<re_irc> <> (I also have thoughts on how to check/verify this, e.g. having a "schema digest" or "schema checksum" you can generate and send on first contact, but nothing implemented there)
<re_irc> <> We wrote a tool we call binary compatibility which parses all our structures and ensure that changes we do to messages are always binary compatible
<re_irc> <> And fills a database of the structure formats
<re_irc> <> And then exhastively compares all versions
<re_irc> <> If any of that is upstreamable to postcard (or with support for postcard), I would be very happy to have it
<re_irc> <> With test vectors for each version so we send them between message versions
<re_irc> <> even if it's an external tool I can link to in the README or docs
<re_irc> <> We accidentally broke our OtA once
<re_irc> <> That stuff makes you do stuff :P
<re_irc> <> Never Again ™️
<re_irc> <> Indeed
<re_irc> <> Maybe we will clean it up and release it, but for now it's the monster that lives in our CI
<Lumpio-> Alright
<Lumpio-> I have a thing where I update the main firmware on startup every time to match the expected one so that should be fine
<Lumpio-> But it makes me a bit wary of using postcard in the bootloader itself, that doesn't get updated
<re_irc> <> Hello, I'm trying to use probe-run on a "ItsyBitsy M4 Express" but did not succeed yet (it's working with cargo-hf2). I followed the tutorial there
<re_irc> <> Do you know if it's possible ?
<re_irc> <> should be. the folks over in the seem to be having a bit of trouble with probe-rs at the moment, maybe best to check out the conversation there?
<re_irc> <> Thanks, I'll check there
<re_irc> <> That's fair, postcard (explicitly states that it) doesn't handle schema versioning, etc., at least not yet.
<re_irc> <> So it would be up to you to verify that, such as a manual "schema version" number or similar.
<re_irc> <> Or, a tool like Korken described.
<re_irc> <> One relatively easy way to do this (in CI) is to use a snapshot testing tool like `insta`, and basically take a reasonable sample size of common messages that should not break
<re_irc> <> That way you at least get a notification when a change occurs, without having to write a lot of boilerplate-y tests.
<re_irc> <> (if you have a known-good schema, you don't even need to review the initial test run, just accept-all-new-snapshots, which is a mode the insta tool supports)
<re_irc> <> (this is not as robust as what was described above, and is only as exhaustive as you make it, but it's a good "quick win" for relatively low total effort)
<re_irc> <> This technique in general (snapshot or "golden master" testing) is also a really good way to detect regressions when significantly changing a codebase without tests, btw.
<re_irc> <> take snapshots - make changes, make sure nothing unexpected changed
<re_irc> <> it's one of the techniques I use a lot when helping out on consulting projects, especially on teams that have not historically had a good practice of testing :)
<re_irc> <> they are often fragile though, so ditching them for more robust tests is always a good thing - if/when you ever get time. But they have a fond place in my toolkit as a useful duct-tape approach :)
<re_irc> <> `/soapbox`
fabic has quit [Ping timeout: 268 seconds]
cr1901 has quit [Ping timeout: 252 seconds]
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 252 seconds]
GenTooMan has quit [Ping timeout: 260 seconds]
DepthDeluxe has joined #rust-embedded
GenTooMan has joined #rust-embedded
GenTooMan has quit [Ping timeout: 265 seconds]
DepthDeluxe has quit [Quit: Leaving]
DepthDeluxe has joined #rust-embedded
DepthDeluxe has quit [Remote host closed the connection]
DepthDeluxe has joined #rust-embedded
DepthDeluxe has quit [Remote host closed the connection]
DepthDeluxe has joined #rust-embedded
DepthDeluxe has quit [Remote host closed the connection]
DepthDeluxe has joined #rust-embedded
<re_irc> <> dirbaio: (or anyone else who knows) did you have trouble with `type parameter \`T\` is part of concrete type but not used in parameter list for the \`impl Trait\` type alias\``? I get this error when I'm using `feature(type_alias_impl_trait)`. Not sure how to work around it.
<re_irc> <> uh can you post full code?
<re_irc> <> or playground
<re_irc> <> ```rust
<re_irc> <> type Error = Infallible;
<re_irc> <> impl DrawTarget for Display {
<re_irc> <> #[rustfmt::skip]
<re_irc> <> async embedded-graphics? :o
<re_irc> <> I'm trying
<re_irc> <> hm what's the trait definition?
<re_irc> <> ```rust
<re_irc> <> pub trait DrawTarget: Dimensions {
<re_irc> <> type Error;
<re_irc> <> type FillFuture<'a>: Future<Output = Result<(), Self::Error>> + 'a
<re_irc> <> It should really be a bit more generic than that - but I'm just trying to get this specific case working.
<re_irc> <> hmm I think it's so the returned FillFuture depends on the type of `I`
<re_irc> <> But being able to pass in arbitrary iterators is kinda essential.
GenTooMan has joined #rust-embedded
<re_irc> <> aah could
<re_irc> <> so maybe something like this
<re_irc> <> type Error;
<re_irc> <> ```rust
<re_irc> <> type FillFuture<'a, I>: Future<Output = Result<(), Self::Error>> + 'a
<re_irc> <> pub trait DrawTarget: Dimensions {
<re_irc> <> I'll give it a go.
<re_irc> <> and maybe even `where I: 'a`
<re_irc> <> GATs become very cursed very fast :S
<re_irc> <> That does work!
<re_irc> <> I wonder how they will solve that problem in the compiler. It looks wierd that there is a free unused type parameter on the associated type.
<re_irc> <> Maybe that's the way it has to be
<re_irc> <> on the trait definition it's OK for GAT params to be unused
<re_irc> <> you can do `type Foo<T>;` just fine
<re_irc> <> it's in the trait impl where they have to be used
<re_irc> <> it's not mandatory to use them, an impl can do `type Foo<T> = u32;` if it wants the type to be the same regardless of the param
fabic has joined #rust-embedded
<re_irc> <> and with TAIT the "use" can even be "implicit" for extra mega curse
<re_irc> <> in your trait above it's definitely used, but implicitly
<re_irc> <> 👻
<re_irc> <> Oh I suppose I'm saying "given this type parameter in addition to the lifetime parameter". How strange.
<re_irc> <> yeah
<re_irc> <> "async fn in traits" supposedly will desugar to all this TAIT cursed code
<re_irc> <> so you won't have to write it by hand
<re_irc> <> For `async` in trait I guess they will want these type parameter to be implicit. That's going to be fun.
<re_irc> <> hm I think the plan is so you don't even have to write the associated type
<re_irc> <> there is an associated type but "implicit"
<re_irc> <> yeah that's what I mean. It will generate the required associated type(s).
<re_irc> <> so the above trait will be simply
<re_irc> <> ```rust
<re_irc> <> type Error;
<re_irc> <> pub trait DrawTarget: Dimensions {
<re_irc> <> async fn fill_contiguous<'I>(&mut self, area: &Rectangle, colors: I) -> Result<(), Self::Error>>
<re_irc> <> which is totally sane
<re_irc> <> :D
<re_irc> <> :)
<re_irc> <> But for now... we don't have that luxury :)
<re_irc> <> yea..
fabic has quit [Ping timeout: 265 seconds]
DepthDeluxe has quit [Ping timeout: 268 seconds]
GenTooMan has quit [Ping timeout: 265 seconds]
GenTooMan has joined #rust-embedded
gsalazar has quit [Ping timeout: 268 seconds]
<re_irc> <> Hey dudes. Looking for thoughts and spears on this HAL periph module layout. I've been using and tweaking it for a bit. Overall, uses enums, structs, and where possible deref owned regs. Minimal abstractions, and tight coupling with RMs. Could be adopted to arbitrary MCUs. Of note, I use a separate Config struct with a default implementation for more complex cfgs
<re_irc> <>
<re_irc> <> room meeting time! i have opened my laptop for the first time especially. agenda's, please add anything you'd like to discuss and we'll start in 5min
<re_irc> <> before we start: friday's stm32 discussion became too heated; I'm sorry I wasn't around to moderate, and thanks to everyone who stepped in and helped lower the temperature. Remember our code of conduct asks you to be kind, respect differences of opinion and different technical trade-offs, and to keep unstructured critique to a minimum; of course you can discuss different approaches to HALs here but please...
<re_irc> ... try to turn it down before it escalates to arguments. It's not OK to outright dismiss someone else's project as completely broken or requiring a total rewrite to be useable; those opinions aren't helpful, don't lead to productive conversations and are best kept to yourself. We keep this channel pretty open to a wide range of discussions and I think it encourages a lot of collaboration and new ideas and developments, but it...
<re_irc> ... must remain friendly and welcoming.
cr1901 has joined #rust-embedded
<re_irc> <> it's clear a lot of people are invested in good stm32 support for rust, and I don't want to discourage that at all, but hopefully we can work together more constructively towards it, even if that means different projects taking different approaches
<re_irc> <> it might not help that I run both stm32-rs and am on the wg core team, so if someone does have an issue with that please message me separately, though so far I have stayed pretty far out of any HAL projects
<re_irc> <> I think the discussions touched on HAL implementations in general, not just stm. I agree fully with the toning down, but I'd like to ensure that we don't just stop talking about this as it seems to be a long standing, recurring problem
<re_irc> <> perhaps felt less acutely for other families? I didn't notice mention of anything except stm32 on friday though I was driving to scotland at the time
<re_irc> <> anyway, I agree it's a discussion worth continuing, but let's get through some of the other agenda items quickly first
<re_irc> <> I've seen cross-advertising on crates work well. Along the lines of "if (e.g. stm32-rs) doesn't meet your needs, you might want to check out ...."
<re_irc> <> oop sorry
<re_irc> <> from last week we had, and it looks like using the deref impls worked well, are there any remaining concerns there?
<re_irc> <> I'm about half way through implementing it
<re_irc> <> I have a "slight" concern which is
<re_irc> <> tldr: why does everyone in the ecosystem do `impl MyTrait for &mut T` instead of the Deref thing
<re_irc> <> right, is this just a "no one's had this clever idea yet" situation or is there some secret terrible downside?
<re_irc> <> Yeah, I haven't seen any crates do auto-impls for all types that deref into something else.
<re_irc> <> exactly
<re_irc> <> the only downside I can think of is "implementors can no longer impl Deref themselves"
<re_irc> <> which doesn't seem that bad for the embedded-hal use case
<re_irc> <> but still, it's interesting
<re_irc> <> or another is
<re_irc> <> I am a bit worried that there could be some other unforeseen consequences, I'll need to do some experimentation
<re_irc> <> implementors can't impl Write for both Uart and Box<Uart>
<re_irc> <> there's some brief discussion at
<re_irc> <> as in, it becomes impossible to impl deref for a struct that also impls the e-h trait?
<re_irc> <> yea
<re_irc> <> oh the error message quality is an interesting one
<re_irc> <> that's 3.5 year old though, maybe it has improved
<re_irc> <> yea, and if it hasn't maybe it could
<re_irc> <> does seem otherwise more useful than only impling it for &mut
<re_irc> <> I don't think we should add something that would forbid someone to implement a Trait on their own type...
<re_irc> <> why would a HAL want to impl Deref though?
<re_irc> <> Not sure, but seems besides the point
<re_irc> <> Uart drivers are not smart pointers
<re_irc> <> so the only reason is to impl Deref to do "fake inheritance"
<re_irc> <> which is considered a bad practice
<re_irc> <> still, adding a implementation for `&mut` gives the same thing without restricting users
<re_irc> <> Deref allows using Box<Uart> and others
<re_irc> <> that was the original motivation
<re_irc> <> though on embedded I guess none of those are very common compared to &mut
<re_irc> <> it'd also work with these "alternative Boxes" such as the ones from heaples::pool or atomic_pool
<re_irc> <> you mean passing Box<Uart> for a type that expects `: Uart` ?
<re_irc> <> wanting to box a Uart like that is uncommon though
<re_irc> <> yeah, why not pass the Uart itself ?
<re_irc> <> 🤷
<re_irc> <> Yes, using Deref and DerefMut allows you to do Driver::new(Box<Thing>) as well as Driver::new(&mut Thing) interchangably, with the downside that nobody can implement their own Deref on Thing anymore
<re_irc> <> Maybe for Rc or Arc but they don't implement DerefMut
<re_irc> <> Hi all, I am a long time embedded développer in C, discover about Rust few month ago, and love it more everyday... I started by doing the PAC for Wiznet W7500P cm0 with hardware Ethernet Mac/phy/IP stack. No I am looking to do the embedded-hal and nal implemention for it. Can someone point me to an exemple or documentation ? Thanks
<re_irc> <> I'm leaning towards the Deref route myself, seems like a nice gain with minimal downside, especially since we sort of know the implementation set
<re_irc> <> hi Georges Palauqui , welcome! we're in the middle of a weekly meeting atm, could you wait 40min to discuss that?
<re_irc> <> Yes sure, sorry
<re_irc> <> thanks :)
<re_irc> <> I will .await() ;)
<re_irc> <> are there any std smart pointers besides Box that it works with?
<re_irc> <> still potentially neater than only impling for &mut and I guess it would be a breaking change to add this later, too
<re_irc> <> almindor: the error messages might not be a very minimal downside
<re_irc> <> Good point!
<re_irc> <> seems worth at least seeing if the error messages are any better these days then
<re_irc> <> I still think that we should not fiddle with user defined types if not required, but that is just an opinion
<re_irc> <> as in, not overly restrict them just because they impl our trait?
<re_irc> <> adamgreig: yep
<re_irc> <> if we didn't do this (and say only impl for &mut) the HAL author could still impl for Box<T> or DerefMut if they want, but end users wouldn't have a blanket impl that guarantees it would work for anything impling the trait
<re_irc> <> And the error message for that would also be a bit confusing
<re_irc> <> but if that only affects Box maybe it's not a huge concern
<re_irc> <> Can anyone show a use case for putting a smart pointer in a driver?
<re_irc> <> i.e. get a conflicting implementations just because you implemented DerefMut
<re_irc> <> I mean, we could instead write a blanket impl for Box and &mut
<re_irc> <> indeed, errors suck
<re_irc> <> with blanket impl: `error[E0277]: the trait bound `u32: DerefMut` is not satisfied
<re_irc> <> without blanket impl: error[E0277]: the trait bound `u32: Traity` is not satisfied
<re_irc> <> thanks
<re_irc> <> errors are still OK with the `&mut T` blanket impl
<re_irc> <> soooo
<re_irc> <> maybe we found The Downside?
<re_irc> <> would a second blanket impl for Box<T> provide most of the use case of DerefMut?
<re_irc> <> it won't cover the heapless or atomic_pool boxes..
<re_irc> <> and using THE alloc Box is not that common
<re_irc> <> and doing the blanket impl would require adding a `alloc` feature
<re_irc> <> so `embedded-hal` doesn't have a forced dependency on `alloc`
<re_irc> <> ah, yea, wasn't thinking of those ones, hm
<re_irc> <> indeed
<re_irc> <> quite unsatisfactory
<re_irc> <> It's fairly easier for a user to implement it for a wrapper type if needed
<re_irc> <> so maybe just `&mut T`? :P
<re_irc> <> and since this isn't common, I think it's okay
<re_irc> <> yeah
<re_irc> <> seems like `&mut T` is the way to go for now
<re_irc> <> ok, thanks for the discussion everyone!
<re_irc> <> hm, I'm not logged in to github here, would someone mind linking; (the logs of it) to that PR?
<re_irc> <> onit
<re_irc> <> thanks
<re_irc> <> added comment
<re_irc> <> oops :) did we race? :D
<re_irc> <> missing mutex or something
<re_irc> <> anything else on embedded-hal to discuss now?
<re_irc> <> oh, it's worth reminding everyone that was released with the latest changes
<re_irc> <> hopefully we edge ever closer to 1.0.0 itself
<re_irc> <> the other main blocker I see is the Time stuff
<re_irc> <> ( and following comments for those following along at home)
<re_irc> <> it seems tricky, some people don't want or like the overhead of a type that's big enough to represent anything, while others need more precision than the integer Hz or wahtever types permit
<re_irc> <> and mixed compromises seem a bit unwieldy, I guess like DelayMs and DelayUs
<re_irc> <> maybe we should ask the question "is it OK to have traits that are not usable for generic drivers"
<re_irc> <> like
<re_irc> <> this has been kinda discussed before
<re_irc> <> e-h has this "dual goal" thing
<re_irc> <> - standardize HAL apis
<re_irc> <> - allow writing HAL-independent drivers
<re_irc> <> not sure if this falls under the scope here but I heavily dislike the `DelayMs` vs `DelayUs` split and the fact that both are already generic over `<T>` for various int types
<re_irc> <> the problematic traits fulfill goal 1 but not goal 2
<re_irc> <> almindor: I've also run into the unwieldiness of this.
<re_irc> <> having readily accessible delay_ms(5) and delay_us(100) seems really convenient to me, but the mechanism is a bit messy
<re_irc> <> IMO all traits should fulfill both
<cr1901> The split doesn't bother me personally, but I've also been bitten by needing to represent times using large ints and try to avoid it
<re_irc> <> wouldn't it be possible to do both by definition or is it a type memory size thing? I don't think having implementation of DelayX<u8> is really useful for example
<re_irc> <> It feels counter to the way that most of the rust ecosystem that time there is represented by rare integers.
<re_irc> <> it's a major pain if a driver implementor expects one and the HAL doesn't have it!
<re_irc> <> so IMO we should invent some mechanism to constraint the `Time` type on these traits
<re_irc> <> and if that's going to take too long and we don't want to delay 1.0, temporarily remove these traits
<re_irc> <> you mean that anyone needing a Delay would have to use 0.x hal for that?
<re_irc> <> only temporarily
<re_irc> <> well DelayMs/DelayUs are *not* broken
<re_irc> <> from my point of view, a goal of e-h is interoperability. I see that as interoperability for drivers with different drivers, but also to some degree interoperability with different hardware, so code does not need to be completely rewritten
<re_irc> <> I still find DelayMs/DelayUs super awkward too, ideally they would use the same mechanism as the others
<re_irc> <> I disagree, I had a case when a driver expected DelayUs but the hal simply didn't implement it. I think that's a broken relationship
<re_irc> <> like hypothetically `delay(` sort of thing?
<re_irc> <> Has there ever been a proposal to spit e-h into a separate crate for each peripheral catagory?
<re_irc> <> adamgreig: What would be nice is some sort of custom suffix macro, so instead of `Duration::from_millis(20)` you can do `20ms`. Very much not planned in Rust yet.
<re_irc> <> almindor: you could argue that's a HAL issue, but yes the current DelayMs/DelayUs don't make it easy for the HALs to implement them
<re_irc> <> we discarded it because of the overhead without clear benefit
<re_irc> <> My take is that the e-h traits should be singular and sort of required, these two delays break that and make these kind of mismatches easy to occur
<re_irc> <> yeah I fully agree!
<re_irc> <> yea, the only good thing about DelayMs and DelayUs is they don't suffer the "how to represent time" issue because it's in the name, but the awkwardness of implementation limits the utility
<re_irc> <> if we had solved the "how to represent time" problem satisfactorily then we'd just have `delay(t: Time)`?
GenTooMan has quit [Ping timeout: 240 seconds]
<re_irc> <> I agree, time representation would solve this into a singular Delay trait
<re_irc> <> Do you not use `core::time::Duration` because of its size?
<re_irc> <> yeah it's super big, and it represents time in seconds+nanoseconds, so you often need multiply/divide by 1e9 which is expensive and totally unnecessary
<re_irc> <> (u64+u32 for core Duration)
<re_irc> <> which is a shame because if it was suitable it would be a really obvious choice
<re_irc> <> (though whatever we end up with would probably be able to impl From/Into Duration I guess)
<re_irc> <> I'm sure people have rolled their own `Duration` types before. Has the advantage that it is familiar to users of `std`. Can't remember where I saw it.
<re_irc> <> is microsecond the smallest unit we care about?
<re_irc> <> (Where the internal representation is different)
<re_irc> <> there are definitely use cases that require nanoseconds
<re_irc> <> gets tricky with timers where the "native" time unit is really clock cycles, and some applications only want to think about units of frequency instead of time
<re_irc> <> Ah so part of the problem here is that there isn't a "one size fits all" time type: it depends on the range and resolution you want.
<re_irc> <> what about a "Duration" trait?
<re_irc> <> seems like the split is between nano (including micro) and milli(including seconds)
<re_irc> <> something like
<re_irc> <> enum Error {
<re_irc> <> ```rust
<re_irc> <> Overflow
<re_irc> <> }
<re_irc> <> I think that the ability to make and release breaking changes to one part without needing to bump the version number for everything else is a clear benefit. It doesn't necessarily outweigh the overhead though
<re_irc> <> yea, there's a summary of a few implementation choices in
<re_irc> <> Duration could have multiple impls that are internally u8, u16, u32... depending on the need
<re_irc> <> and the HAL would convert to the type it wants
<re_irc> <> without forcing using bigger-than-necessary uXXs
GenTooMan has joined #rust-embedded
<re_irc> <> it can probably be made cleaner with From/Into idk
<re_irc> <> yeah, sounds good at first but each breaking change would effectively create splits in the community and contribute to chaos
<re_irc> <> Could be ‘Ms<T>’, ‘Us<T>’, so you can store different ints
<re_irc> <> One of the advantages of a trait like this is that you can implement it for `core::time::Duration`, and tell beginners to use this. Then if/when people need more control of memory usage, they can use a different impl.
<re_irc> <> You can go one further and have something like `10u8.seconds()` so you can specify the actual storage type? many people would misuse with the wrong type tho and make it unnecessarily costly
<re_irc> <> you'd have `Seconds<T>`, `Milliseconds<T>`, `Microseconds<T>`, all implementing `Duration`
<re_irc> <> `core::time::Duration` could also impl it yup
<re_irc> <> and also `embedded-time` etc
<re_irc> <> I feel like that would make sense to someone seeing it for the first time.
<re_irc> <> maybe it's too complex though
<re_irc> <> (I'm not qualified to comment on the technical aspects, but from a developer point of view it sounds nice)
<re_irc> <> and there's another problem: HALs may want to convert to arbitrary tick rate
<re_irc> <> for example if the TIM3 clk runs at 3Mhz
<re_irc> <> What would the duration trait be?
<re_irc> <> it has to first convert to microseconds, then to 3mhz ticks
<re_irc> <> You'd have an intro saying "Because time is such a critical part of embedded functionality, there are lots of different use cases and needs ... which is why there are so many different ways of representing time"
<re_irc> <> You could probably support that with a `fn tick_rate(clock: &Clock)` or such?
<re_irc> <> converting directly would have better precision
<re_irc> <> would you have a `HfClk<T>` and a `LfClk<T>` that sleep for ticks rather than wall time. Then best-effort conversion between them with docs explaining which are more accurate?
<re_irc> <> ```rust
<re_irc> <> yea, nice
<re_irc> <> in principle `delay(5.ticks())`
<re_irc> <> `timer.set_period(100.ticks())`
<re_irc> <> no
<re_irc> <> :'(
<re_irc> <> the user would do `delay(Milliseconds(100))`
<re_irc> <> `Milliseconds` would impl `Duration`
<re_irc> <> and *the HAL* would call `as_ticks_uXX` with the tick rate its hardware timers have
<re_irc> <> Sounds like a good idea to me
<re_irc> <> and write that to the HW register
<re_irc> <> But sometimes you don't actually care about wall time. For example rp2040 has an integer divide that takes 8 cycles.
<re_irc> <> sure, that conversion is how the HAL driver would work out ticks from a duration in ms
<re_irc> <> you can also add helpers to primitive types `u32::into_millis()` or such so you can `delay(10u32.into_millis())`
<re_irc> <> I wonder if you could additionally allow a Duration to exist that only knows what it is in ticks, not in seconds, though
<re_irc> <> for "delay for 8 clock cycles" you'd use ASM, not the `Delay` trait
<re_irc> <> adamgreig: That is the question.
<re_irc> <> allowing delaying for a number of ticks for example
<re_irc> <> sure, for 8 cycles, but setting a timer period to a number of ticks is more useful
<re_irc> <> should we have the same for frequencies (`Hz<T>`, `Khz<T>`, `Mhz<T>`)?
<re_irc> <> Yes you could. This is an alternative strategy. I thought it would be nice though if you could combine cycles and wall time into a single interface. It's not necessary for the reasons you mention, just whether it would be a 'nice to have'.
<re_irc> <> IMO "frequency" types are broken
<re_irc> <> you can't delay for 0.7s or for 2s
<re_irc> <> uh
<re_irc> <> this way, the `tick_rate` arguments would be types
<re_irc> <> some APIs are going to want frequency types though (timer setup for instance), and they'l have the same problem about how to represent them
<re_irc> <> "delay for clock cycles" is very hardware-speicfic, I don't see how a HAL-independent driver would want that
<re_irc> <> maybe we can duck that in e-h
<re_irc> <> hmm, that's true, yea
<re_irc> <> "clock cycles of which clock"? Core clock? SPI clock? APBx clock?
<re_irc> <> I feel like I'm kinda bikeshedding here so might stop now
<re_irc> <> the HAL-independent driver doesn't even know what APBx is :D
<re_irc> <> I was more thinking of an end user interacting with the hal directly, where being able to write a delay as a number of cycles instead of a number of ms might be more convenient or optimised better
<re_irc> <> why is that? aren't they the same concept as time types?
GenTooMan has quit [Ping timeout: 260 seconds]
<re_irc> <> I meant using frequency as a "duration" is broken
<re_irc> <> oh yeah, sure
<re_irc> <> because you can't represent 0.8s or 2s
<re_irc> <> Should the `as_ticks` methods round up or down?
<re_irc> <> ofc it makes sense for representing a frequency
<re_irc> <> asking the hard questions there, I see :D
<re_irc> <> No idea :D
<re_irc> <> round to nearest ;)
* re_irc gotta go. nice discussion people! I think a proposal around this would be very interesting
<re_irc> <> I have to head out and we're over time for the meeting so I'll duck out here, but yes some good ideas I think!
<re_irc> <> ((value * 2) + 1) / 2). Or on M0 use software divide
<re_irc> <> hello overflow :P
<re_irc> <> thanks as usual for all the discussions!
<re_irc> <> to very briefly cover the remaining agenda items, bsaically I'll get to them when I'm back from holiday :P
<re_irc> <> I'll try and get a draft of the send/sync/environment stuff together and solicit feedback, hopefully we can polish it into something useful
<re_irc> <> I'm gonna try to put together an RFC of the Duration trait
<re_irc> <> not sure how to impl as_ticks_uXX
<re_irc> <> it seems like as_ticks_u32 needs to temporarily use an u64 🤔
<re_irc> <> not sure if that's acceptable
<re_irc> <> oh and it doesn't work for traits that want to *return* a Duration
<re_irc> <> like Capture
<re_irc> <> 💩
<re_irc> <> it needs `from_ticks_uXX`
<re_irc> <> not pretty :S
<re_irc> <> Speaking of e-h api design, I have a question about `InputPin`
<re_irc> <> The methods return a `Result` so that they can be used with a gpio expander over i2c or something, right?
<re_irc> <> yep
<re_irc> <> But the methods take `&self`, not `&mut self`
<re_irc> <> So would a gpio expander need to use a `RefCell` or somthing?
<re_irc> <> probably :S
<re_irc> <> maybe it's not that bad
<re_irc> <> because
<re_irc> <> typically a GPIO expander has multiple pins
<re_irc> <> so the struct implementing InputPin won't have an `inner: MyExpander` inside anyway
<re_irc> <> it'll have a `&'a RefCell<MyExpander>`
<re_irc> <> so multiple pins can refer to the same expander
<re_irc> <> I wonder what the downside of changing it to `&mut self` are though
<re_irc> <> can't think of any
<re_irc> <> so maybe it's worth it
<re_irc> <> Plus I think that HALs could impl `InputPin` for `PinStruct` and `&PinStruct`
<re_irc> <> HALs could also do that for `OutputPin`, but I am not sure if that is a good idea
<re_irc> <> If a driver holds something that impls `OutputPin`, it probably assumes that there is no (safe) way for someone else to change the state of that pin
<re_irc> <> What about `StatefulOutputPin?`
dcz_ has quit [Ping timeout: 260 seconds]
<re_irc> <> A gpio expander driver that impls `StatefulOutputPin` would probably keep track of the state in the driver, so it would be able to get the state with `&self`
<re_irc> <> an expander driver will need some form of interior mutability in any case to be able to expose individual pins separately. So `&self` is not a problem for `InputPin` and `StatefulOutputPin`
<re_irc> <> I have a data structure question: does anyone know of a data structure for tracking dirty regions of a display. What I'm thinking of doing is just keeping track of dirty rectangles and unioning whenever there is a draw that changes the screen contents. But I wonder if there is a better way..
<re_irc> <> I've been playing around with that in my crate, if you're interested
<re_irc> <> that, or a list of up to N (maybe <4) rectangles
<re_irc> <> or maybe split it in like 64x64 tiles and for each tile track one "dirty" bit
<re_irc> <> Hmm I quite like the dirty bit approach: will try that :)
<re_irc> <> Got any harsh words about my current radio mess? :P
<re_irc> <> oh hai Oddstr13
<re_irc> <> oh noes, I have awoken the sparky 🙈
<re_irc> <> i'm afraid it's still unsound 😭
<re_irc> <> the buffer is owned by Radio
<re_irc> <> you can still start a DMA into the buffer, and then deallocate the Radio
<re_irc> <> right
<re_irc> <> unfortunately sound DMA is ultra hard
<re_irc> <> is there a deallocation hook I could mess with? Rust being like «Hey, so, uh, you're about to go poof, fix your shit please»?
<re_irc> <> you mean implementing `Drop`?
<re_irc> <> That does indeed sound like what I'd want
<re_irc> <> unfortunately in Rust it's possible to deallocate things without running drop
<re_irc> <> and without unsafe
<re_irc> <> with mem::forget and others
<re_irc> <> there's really good discussion here
<re_irc> <> the tldr is it's still an unsolved problem
<re_irc> <> so, what you're saying is I'd have to figure out how to give radio a borrowed 'static packet?
<re_irc> <> yeah, the buffers have to be static
<re_irc> <> so if you leak them the memory is still there and DMA doesn't overwrite something else
<re_irc> <> but that makes the API super unergonomic to use
<re_irc> <> there's the `embedded-dma` traits that specify more formally what the requirements for a dma buffer are
<re_irc> <> but in the end it's just `&'static mut` and then stuff that requires alloc such as `Box` and `Vec`
<re_irc> <> I implemented a `split()` method in the pcf857x I2C GPIO expander driver using a `RefCell` for interior mutability.
GenTooMan has joined #rust-embedded
<re_irc> <> Oddstr13: if you want clues for the dma stuff you can look at how it’s implemented in the spis, twis and i2s modules, there are examples of how to use them using the Transfer api in the examples folder too. Let me know if you want me to clarify anything
<re_irc> <> I'm not an expert on embedded-time, but from what I remember it already supports everything you mentioned in a very flexible way. Maybe it's worth looking into it, before reinventing the wheel.
<re_irc> <> I also don't think it's possible to find an easy solution to this problem. It's complicated, so any API that aims to support all or most use cases won't be trivial.
<re_irc> <> Embedded time doesn't support 16-bit timers unfortunately which is rather critical for some microcontrollers.
<re_irc> <> It isn't trivial to add either because there are a lot of assumptions about 32-bit integers being used.
<re_irc> <> Frequency types are not broken IMO. They are not really suited to express long durations the same way microseconds are bad at expressing frequencies at or above 1MHz (or ns <-> GHz). It depends on your desired range and resolution.
<re_irc> <> That is indeed a problem. I'd still think it's probably better to improve/expand embedded-time instead of solving it again in e-h. Especially since RTIC and a lot of hals already use it.
<re_irc> <> Having yet another duration type won't do any good IMO.
<re_irc> <> I think a duration type in EH makes a lot of sense.
<re_irc> <> 1. Embedded time has a very high bus factor, just one owner. The owner has not been receptive to adding another maintainer (which is absolutely ok, his project, his rules), but for large scale use I would like an embedded duration type to not be entirely dependent on a single person for maintenance.
<re_irc> <> 2. Embedded time takes a long time to compile (3.5s vs 0.5s) because the scope is large. Limiting the scope and adding a duration type to embedded time would make it a more approachable dependency.
<re_irc> <> To be clear, I like embedded time, and I use it, but I think it has some limitations that could be resolved by moving some of that work to embedded-hal
<re_irc> <> the idea is to let HALs use any duration type, while still allowing any HAL to work with any driver
<re_irc> <> so it's a duration *trait*, not a duration *type*
<re_irc> <> for most cases the duration type would actually be something like u32
<re_irc> <> yea, or something like Microseconds<u32>
<re_irc> <> also wrt. Frequency I think it should be its own trait
<re_irc> <> [newam]( I can see your point. If the owner wants to stay the single maintainer that is problematic.
<re_irc> <> embedded-time uses a duration trait. It's almost exactly what you came up with as far as I can see.
<re_irc> <> hmm something like that
<re_irc> <> that one can only convert to other embedded-time types
<re_irc> <> but yea
<re_irc> <> that Duration trait would be just "any type that can be converted to/from in this standard way"
<re_irc> <> HALs use their preferred duration type
<re_irc> <> for example if it's a 16bit timer their preferred duration type might be "u16, representing tick count of APB2 clock"
<re_irc> <> and that type will impl Duration
<re_irc> <> which requires a "from_microseconds" or something, and the HAL can impl that conversion since it knows the APB2 clock rate
<re_irc> <> korken89: has been doing the rtic impl and working with the author, they would presumably have some comment
<re_irc> <> I'm not an embedded-time fanboy. It's probably not perfect. But it is widely used and tested and should be considered as option or at least used as an inspiration before starting completely from scratch. And the e-h solution should be compatible to minimize friction.
<re_irc> <> the idea is very fuzzy though, I guess I have to try it out and figure out the details
<re_irc> <> hm embedded-time is the one who would have to impl the embedded_hal::Duration to be compatible
<re_irc> <> this is a bit of an excercise in shuehorning, because we want to both use the most efficient primitives possible, while allowing a full range setup in the end
<re_irc> <> I also have RollingTimer::Ticks in groundhog, but afaik I'm the only one who actually uses it :D
<re_irc> <> My approach is "what if bad, but also simple?"
<re_irc> <> Which is about all I can wrap my head around these days.
<re_irc> <> not overengineering stuff is nice 👍️
<re_irc> <> That being said, don't look at the trait bounds on the Tick type 😅
* re_irc looks
<re_irc> <> don't tell me what to do
<re_irc> <> crate name proposal: ticks_for_pricks 😀
<re_irc> <> ` type Tick: RollingSince + Promote + Div<Output = Self::Tick>;`
<re_irc> <> 404: puke emoji not found
<re_irc> <> they're private or something? they're not clickable in rustdo
<re_irc> <> It's mostly just ensuring you don't have overflow when doing conversions to ms/us whatever
<re_irc> <> fancy
<re_irc> <> The core itself just thinks in ticks. That's only really for the .millis_since() interface
<re_irc> <> you may want to make RollingSince, Promote public
<re_irc> <> otherwise there's no way to know what they are from rustdoc
<re_irc> <> Like I said, I'm pretty sure no one actually uses this
<re_irc> <> Other than me in ally my projects
<re_irc> <> I find the interface pretty pleasant in practice though:
<re_irc> <> I also always make my timer a global singleton though, so your tastes may vary.
<re_irc> <> Plus I get to write the Good Code, that Will Definitely Not Make dirbaio Angry:
<re_irc> <> Ignoring the waker ☠