<adamgreig[m]>
are you using the F723 disco board?
<adamgreig[m]>
that note from the docs is just saying "even if the pin is being driven high, if for example the physical pin has been shorted to ground then it won't be a high voltage [because it's shorted to ground]", it's probably not relevant to this
Foxyloxy_ has joined #rust-embedded
<paulyoung[m]>
adamgreig: Yes, I’m using the discovery board
<adamgreig[m]>
and what pin are you driving / is your code anywhere?
<adamgreig[m]>
I guess it's just a mistake in the user manual, it's a shame they don't seem to have a schematic available
Foxyloxy has quit [Ping timeout: 256 seconds]
<adamgreig[m]>
oh, they do have a schematic, which pin are you driving?
cr1901 has quit [Read error: Connection reset by peer]
cr1901 has joined #rust-embedded
crabbedhaloablut has joined #rust-embedded
emerent has quit [Ping timeout: 256 seconds]
emerent_ has joined #rust-embedded
emerent_ is now known as emerent
Lumpio[m] has joined #rust-embedded
<Lumpio[m]>
* sigh * Why do chip companies never hire software developers who are actually good at software and API design to do their official software
M9names[m] has joined #rust-embedded
<M9names[m]>
if you are actually good at software and API design, why would you work for a hardware company when you can get better career progression and significantly better pay working elsewhere?
<Lumpio[m]>
I'm not blaming the software people, I'm blaming the hardware companies lol
<Lumpio[m]>
If they can't attract skilled software people maybe they should reconsider their compensation
<K900>
You don't need to allocate to print a stacktrace
<K900>
If you're just printing it as you go and not building up a string in memory
<lockna[m]>
Yeah, thats true, but why focus on printing a stacktrace when "more important" things are outstanding to be done
<K900>
Because if the more important things crash, you want to know why?
<K900>
Like it seems reasonable to me unless you want to have an SWD probe connected all the time
<dsvsdveg[m]>
this tutorial is for all rasp berry pi ?
<dsvsdveg[m]>
1 2 3 4 .. ?
<lockna[m]>
> The code written in these tutorials supports and runs on the Raspberry Pi 3 and the Raspberry Pi 4.
<K900>
1 and 2 will be harder
<K900>
Because armv7 is weird
<K900>
And armv8 is... less weird
vollbrecht[m] has joined #rust-embedded
<vollbrecht[m]>
also a good look, if you are new to operating systems is to have a look at https://github.com/mit-pdos/xv6-riscv while its C based its good for the fundamentals. You can try to adapt it to rust.
<dsvsdveg[m]>
vollbrecht[m]: i love this but this only have code, tought will found some docs
<K900>
Also if you don't specifically need RPi for some reason, maybe try a different target
<K900>
Like x86_64 UEFI
<K900>
The RPi boot chain is... interesting
<dsvsdveg[m]>
K900: which one support x86_64 ?
<K900>
JamesMunns[m]: This for example
<K900>
And you can boot it on your actual PC afterwards
<dsvsdveg[m]>
K900: yeah i see i but i need to use rasp berry sadly
<dsvsdveg[m]>
this would be experience for me to be more comfortable with rasp berry pi
<dsvsdveg[m]>
how much does it cost the rasp berry 3 or 4 ?
<K900>
Generally if you plan to build other projects on RPi later, you don't want to write your own kernel
<K900>
You want to use Linux
<K900>
Especially if you want to do anything with graphical output
<K900>
Because getting that running on not-Linux will be very very very difficult
<dsvsdveg[m]>
K900: yeah but actually what i would do is making a small operate system with basic thing like writing, reading etc
<dsvsdveg[m]>
memory allocator and other thing
<K900>
But then you'll want to run other things on your OS and that's when you will have problems
<dsvsdveg[m]>
K900: in truth I don't plan to run things on the OS that I would like to do but on the other hand I will switch to Linux for things like that :)
<dsvsdveg[m]>
so if i understand well most of the implement is done by refering to aarch64
<dsvsdveg[m]>
like if else block and matcin in increasing and decreasing value
<K900>
`if` and `match` are expressions in Rust
<K900>
They can be used anywhere an expression can be used
<dsvsdveg[m]>
K900: yeah but it's first time i saw tis in something like value +=
<dsvsdveg[m]>
s/tis/this/
<burrbull[m]>
even loop is expression
<vollbrecht[m]>
For your rpi specific boot-system you might be in luck, because microsoft open-sourced [threadx](https://github.com/azure-rtos/threadx) the rtos that is running the bootchain on the pi's. Though keep in mind the rpi foundation did not open-source its fork of it that is currently in use. But now the path for that is theoretical open
<K900>
Technically there's already a reverse engineered libre bootchain for RPi3 at least
<vollbrecht[m]>
K900: yeah, if we got the rpi version of threadx, it would be really cool. This would also help better understand write an alternative for it if one wishes to do so.
<vollbrecht[m]>
* better understand how to write an
<jessebraham[m]>
That one is easy enough to work around, albeit ugly. But there are a number of arrays of registers for which we have no way of knowing the length of now,m which is more problematic.
<vollbrecht[m]>
* better understand how to write an, * do so - that will not be jank in the end.
<jessebraham[m]>
Eg) `self.aes.key_.len()` is no longer possible
<jessebraham[m]>
Oh wow, that was fast! Thank you for that!
<jessebraham[m]>
I'm just wrapping up work for the day, but can take a peek in the morning!
StephenD[m] has quit [Quit: Idle timeout reached: 172800s]
lulf[m] has quit [Quit: Idle timeout reached: 172800s]
<adamgreig[m]>
hi @room, it's meeting time again! agenda is at https://hackmd.io/pEbrgzrCTOiNmB2Ld-yaCQ, please add anything you'd like to announce or discuss, we'll start in a few mins
<dirbaio[m]>
embedded-hal
<dirbaio[m]>
lots of embedded-hal stuff
ithinuel[m] has joined #rust-embedded
<ithinuel[m]>
Hello adamgreig
<adamgreig[m]>
we asked for feedback, we got it
<adamgreig[m]>
at least it's before actually releasing 1.0 :P
<adamgreig[m]>
but yea, pick any of them you want to discuss I guess, I saw a bunch came in
<JamesMunns[m]>
I'll have to run out partway through the meeting, but CC adamgreigwriting up an issue for that "call for blog posts" thing I mentioned
danielb[m] has joined #rust-embedded
<danielb[m]>
you guys planning on an rc.2 for some last minute tests?
<adamgreig[m]>
oh yep, shall we discuss it early then so you can be here?
<adamgreig[m]>
well, let's start, couple of announcements first, the main one is the new svd2rust 0.31 release, which changes the syntax to use methods for all registers, i.e. instead of uart.cr.write(|w| ...) it's now uart.cr().write(|w| ...)
<adamgreig[m]>
which is probably going to involve a lot of mechanical changes in HALs, but we expect to be necessary to eventually do away with references to memory mapped structs
<JamesMunns[m]>
(I should be good to stick around until half past or so)
<adamgreig[m]>
thanks jessebraham for finding various related issues which can hopefully be resolved soon :P
<adamgreig[m]>
well, let's do you now, go for it James Munns
<JamesMunns[m]>
I wanted to egg other people into writing blog posts! I've arbitrarily chosen "five years since stable embedded rust" as my excuse.
<dirbaio[m]>
don't want to spend much time on it, it's almost-bikeshedding. if no one has any concerns i'd merge
<dirbaio[m]>
IMO debug_assert! is the right tradeoff, catches bugs when enabled and doesn't slow down when disabled
<adamgreig[m]>
I think it's fine. it's weird for the comment to say it's safe because we know n<=d, because that's only a debug_assert so it might not actually be true
<adamgreig[m]>
but still I agree it's a good tradeoff, just nitpicking comment text
<dirbaio[m]>
I want the async crates to work on beta (to prepare for 1.75 stable), and beta/stable outright disallows feature(), even for already-stable features
<dirbaio[m]>
AFIT feature() hasn't been required in nightly for a while
<dirbaio[m]>
but it's still required in latest xtensa-rust
<adamgreig[m]>
seems OK, I guess it'l probably linger for a while until the first patch release, but that's not a huge problem?
mabez[m] has joined #rust-embedded
<mabez[m]>
We might be able to do a beta release, for this specific case, if that helps? How many folks do you anticipate being stuck on an old nightly?
<mabez[m]>
Might take a few days to prepare, if we want to go down that route
<dirbaio[m]>
(they build from the stable branch but enabling nightly features, so latest xtesna-rust is based off 1.74 stable which is from before marking AFIT as stable)
<adamgreig[m]>
if the only reason to have it at all is xtensa 1.75 and that could be resolved quickly then I guess it's even better if we don't need it, but it doesn't seem too burdonsome
<mabez[m]>
For rust-xtensa this is of course
<dirbaio[m]>
mabez[m]: yeah main motivation for this is xtensa-rust. otherwise I'd just drop support for older nightlies (already have in Embassy)
jannic[m] has joined #rust-embedded
<jannic[m]>
Is it always sufficient to check RUSTC or might RUSTC_WRAPPER be important in some cases?
<adamgreig[m]>
yea, I definitely wouldn't hesitate to drop support for older nightlies
<cr1901>
they build from the stable branch but enabling nightly features <-- in rustc?
<mabez[m]>
cr1901: yeah
<dirbaio[m]>
jannic[m]: `rustversion` and the other similar crates I checked use only `RUSTC`
<danielb[m]>
IMO if we have a stable 1.75-based xtensa out I'll be fine with dropping the feature gates. My issue previously was that we had no alternative
<danielb[m]>
* 1.75-based xtensa rustc out I'll
<dirbaio[m]>
IMO the autodetect is not that burdensome
<cr1901>
Meaning "the stable compiler has nightly features in it" or "nightly compiler from stable branch source code is required"?
<cr1901>
(also, no complaints for dropping older nightlies)
<mabez[m]>
danielb[m]: This would be for the beta period, I will aim to have 1.75 rebased as soon as its merged in the stable branch upstream so we can remove it almost instantly
<dirbaio[m]>
I'd rather merge+release the autodetect now than wait for a xtensa-rustc beta. it's less breaking for the ecosystem
<mabez[m]>
cr1901: the latter, we just build a stable compiler in the nightly channel mode
<cr1901>
ahh
<adamgreig[m]>
maybe merge, release autodetect now as the next rc, and remote it for the 1.0 release?
<mabez[m]>
dirbaio[m]: Okay sounds good to me!
<adamgreig[m]>
s/remote/remove/
<dirbaio[m]>
otherwise xtensa users will be forced to use the beta which might have fun bugs (?)
<dirbaio[m]>
vs xtensa-rust 1.74 is known to work
<danielb[m]>
dirbaio[m]: don't worry we'll have fun bugs regardless 🫠
<mabez[m]>
Yeah I think its best to base releases on stable revisions in general, less work involved and less random breakage for users
<adamgreig[m]>
if we can get rid if it pre-1.0 in exchange for just a few days of xtensa users needing to wait for their 1.75 stable release, great
<danielb[m]>
adamgreig[m]: if the autodetect is in and it doesn't cause problems, will there be an incentive to remove it?
<adamgreig[m]>
nicer to not have a build script if we can avoid it, slows compile times and such
<adamgreig[m]>
not significantly though
<mabez[m]>
danielb[m]: I will vow remove it on the 28th :D
<dirbaio[m]>
downside is tiny, just slightly slower cold builds (doesn't impact warm builds)
<danielb[m]>
mabez[m]: you. will. not.
<dirbaio[m]>
we can keep it for a few weeks after 1.75 is out
<dirbaio[m]>
then remove it
<dirbaio[m]>
(on grounds that MSRV is 1.75 so we're allowed to break nightlies and xtensa-rust 1.74)
<danielb[m]>
* will. not. (please don't work right after christmas 🫠 )
<adamgreig[m]>
so you'd keep it in the 1.0.0 release, but then remove it in 1.0.1 a few weeks later?
<dirbaio[m]>
maybe? or just remove it in 1.0.0 directly
<jannic[m]>
<dirbaio[m]> "`rustversion` and the other..." <- Ok sounds good enough.
<mabez[m]>
I suppose removing the feature is a breaking change so...
<adamgreig[m]>
yea, I'd have thought just remove it in 1.0.0 directly
<dirbaio[m]>
there's also embedded-io 0.6, we're not doing a 1.0 release of that yet
<dirbaio[m]>
* also embedded-io-async 0.6,
<adamgreig[m]>
ok, sounds like let's merge this now but plan to have it gone before 1.0 then?
<dirbaio[m]>
sounds good to me👍️
<dirbaio[m]>
will send a PR to do a rc release soon
<dirbaio[m]>
s/soon/now/
<mabez[m]>
dirbaio[m]: I assume we'd cut a 0.7 release with the nightly features removed too though?
<dirbaio[m]>
s/soon/now with the autodetect/
<dirbaio[m]>
if the only breaking change is removing the autodetect, I'd tag it as v0.6.x
<dirbaio[m]>
a new major version would be annoying, and somewhat pointless since there's been no breaking changes to the traits themselves
<adamgreig[m]>
only affects old nightly users anyway right?
<dirbaio[m]>
yep
<dirbaio[m]>
(and xtensa 1.74 users)
<adamgreig[m]>
do you want to talk about any of the recent e-h issues? there's spi delay granularity, delay using core::duration, renaming get_max_duty_cycle, unifying spi traits, avoiding "errortype", and removing "errorkind" from digital/pwm, but not sure any need discussing in a meeting
<dirbaio[m]>
yeah
<dirbaio[m]>
many are reiterating already-discussed topics tho
<dirbaio[m]>
for example we already decided to not use core::time::Duration because size and expensive maths
<dirbaio[m]>
there's a few with suggestions taht don't really work imo, i've replied to them indivudually
<dirbaio[m]>
* for example we already decided to not use core::time::Duration because size and expensive maths
<dirbaio[m]>
we already discussed it on the latest meeting, right?
<adamgreig[m]>
yea
<dirbaio[m]>
conclusion was to declare it is "configuration", like SPI freq and mode, right?
<dirbaio[m]>
therefore out of scope
<adamgreig[m]>
I think the conclusion is we hope we're making the right bet on the current abstraction being able to support most things, and at least SpiBus is there for others, and maybe in the future we'll have other ways to share SpiBus or handle configuration
<dirbaio[m]>
it still makes me a bit uncomfortable though that we do have a "delay" operation
<dirbaio[m]>
because it's SHOUTING at the user "use me for CS-to-clock-to-CS delays!"
<adamgreig[m]>
configuration probably can't be out of scope of the bus sharing structs forever, especially for common spi things, but even then the cs-to-data timing is kinda specialist and most hardware won't do anything for it
<adamgreig[m]>
hah, yea, I guess so
<bartmassey[m]>
dirbaio[m]: Can we not provide both options by adding a separate`embedded_hal::delay::Delay` and let the user choose? Or am I missing something obvious? (Usually)
<adamgreig[m]>
if we did hypothetically have DelayNs, would you want drivers to use that around every transaction?
<adamgreig[m]>
it seems like the better design in this hypothetical is for the bus-sharing struct to handle it
<dirbaio[m]>
yeah
<dirbaio[m]>
for example with hardware-CS the hardware can often do the delay
<adamgreig[m]>
but we don't have any way for the driver to tell the bus-sharing struct what spi mode, clock speed, or timing delays it needs
<dirbaio[m]>
if drivers start doing the "delay, write, delay" thing
<dirbaio[m]>
SpiDevice impls will have to do "check if 1st and last operations are delays, if so strip them out and configure the hardware to do the delays instead"
<adamgreig[m]>
someone might ask "well, why have delay_us at all", but I feel like its inclusion was reasonably well justified, even if only by "well, everyone can implement it, and some devices need it"
<adamgreig[m]>
dirbaio[m]: they don't _have_ to, do they?
<adamgreig[m]>
they could just assert cs as normal and then do the delay
<dirbaio[m]>
sure
<dirbaio[m]>
but then it's not really hardware-CS
<adamgreig[m]>
"hardware cs" is such a weird concept for bus masters, heh
<adamgreig[m]>
anyway
<adamgreig[m]>
it certainly seems like it shouldn't be part of the transaction. maybe one day we have an extension to the SpiDevice trait that's like SpiDeviceWithConfig and you can pass a bus mode, max clock speed, CS timing, etc to transaction()
<adamgreig[m]>
s/part/inside/, s/of//
<adamgreig[m]>
in the meantime people can use spibus directly, or a custom bus sharing thing that adds the delay, or worst-case use delay_us
<adamgreig[m]>
anything else for e-h?
<dirbaio[m]>
so conclusion = declare it officially as "configuration"?
<dirbaio[m]>
- declare cs delays officially as "configuration"
<dirbaio[m]>
* so conclusion
<dirbaio[m]>
- document that drivers shouldn't use Delay operations for cs delays (in first and last position )
<dirbaio[m]>
* so conclusion
<dirbaio[m]>
- declare cs delays officially as "configuration"
<dirbaio[m]>
- document that drivers shouldn't use Delay operations for cs delays (in first and last position)
<dirbaio[m]>
?
<dirbaio[m]>
nothing else about e-h from me
<adamgreig[m]>
I don't know about declaring them "configuration", what does that even mean? we can say it's out of scope for the SpiDevice trait as-is because we can't think of any good way to include it
<adamgreig[m]>
another Operation in transaction doesn't seem like a good answer even if it was possible (which it isn't on linux)
<dirbaio[m]>
"configuration" as in "it's something the user should configure right (and use an impl that allows them to configure it right)"
<dirbaio[m]>
like SPI frequency or mode
<dirbaio[m]>
ie out of scope of the SpiDevice trait
<adamgreig[m]>
yea, ok, and maybe that means "configure their bus-sharing struct to handle it" since most hardware probably doesn't have anything intrinsic
<adamgreig[m]>
there was a suggestion of a custom chip select OutputPin impl that asserts+delays, and delays+deasserts
<dirbaio[m]>
that's a hack to reuse the e-h-b impls IMO
<adamgreig[m]>
the driver can't enforce that it's used, but can provide it as a wrapper type or something
<dirbaio[m]>
I wouldn't encourage it
<adamgreig[m]>
yea
<dirbaio[m]>
just write your own SpiDevice impl
<dirbaio[m]>
until we add something to e-h-b
<adamgreig[m]>
how? if you're a driver you shouldn't be providing an SpiDevice impl
<adamgreig[m]>
otherwise the end user is stuffed if two drivers both want to you use their incompatible sharing structs
<dirbaio[m]>
the user or the HAL would provide it
<dirbaio[m]>
there's no way to do nanosecond-precision delays through e-h traits right now
<adamgreig[m]>
yea
<adamgreig[m]>
yea, so it's something the driver documents "you must ensure happens" and it's up to the user to sort out
<adamgreig[m]>
be that through a hal, a custom spidevice, a weird outputpin, whatever
<dirbaio[m]>
maybe the Delay trait should support nanosecond resolution though... it still makes me a bit uncomfortable to not have it
<adamgreig[m]>
distinct from the delay operation in spi?
<dirbaio[m]>
yes
<dirbaio[m]>
I mean
<dirbaio[m]>
it's orthogonal
<adamgreig[m]>
the Delay trait, or a new DelayNs, or..?
<GrantM11235[m]>
It seems like there has been a pretty big shift from "HALs shouldn't write their own SpiDevice, just use e-h-b" to "HALs should write their own SpiDevice"
<adamgreig[m]>
GrantM11235[m]: yea, this concerns me a bit, I don't think any hals are likely to write their own spidevice just to provide configurable ns-level delays around cs
<adamgreig[m]>
but I think that doesn't change the conclusion for spi delays
<dirbaio[m]>
adamgreig[m]: iirc there were issues with async default impls, but I think that's back from the TAIT days.
<adamgreig[m]>
would be worth getting it in if it can be done
<adamgreig[m]>
re BasicDelay, I guess the concern is what to do on platforms that can't implement ns-level delays for some reason, document that they'll just round up?
<adamgreig[m]>
dirbaio[m]: you can trivially construct your own DelayNs from a DelayUs
<adamgreig[m]>
and at least then you know it's going to round up horribly
<mabez[m]>
* I would be in favour of renaming `DelayUs`, might be a silly question but whats wrong with just calling the trait `Delay`?
<adamgreig[m]>
mabez: we live in hope of one day having a Duration type that everyone agrees on, and then Delay would use it
<dirbaio[m]>
adamgreig[m]: if BasicDelay is allowed to round up, it's allowed to round up horribly 🙃
<jannic[m]>
What's the use case where a ns delay is useful, and rounding up to µs is fine without causing completely unreasonable delays?
<adamgreig[m]>
it would be more about setting user expectations around what their hal is providing
<dirbaio[m]>
jannic[m]: SPI CS delays
<adamgreig[m]>
it would cause completely reasonable delays I think
<adamgreig[m]>
you want like 20ns and you get 1000ns, for a transaction that's maybe 2us extra time, at 20MHz SPI clock that's 40 clock cycles per say 24-bit exchange wasted
<adamgreig[m]>
it'l work but it's 1/3 the speed
<adamgreig[m]>
but like, maybe that's better than nothing?
<adamgreig[m]>
if we did do that, we could have spi transaction accept delay_ns and round up where required (linux), since most spidevice providers would just require a e_h::Delay which could do ns
<dirbaio[m]>
having things Just Work, even if slower, is better IMO
<therealprof[m]>
Certainly better than nothing. Especially when there're ways around that.
<adamgreig[m]>
and then for now drivers could use delay inside the transaction even if we don't love it, until something better is available
<dirbaio[m]>
if the user cares about perf they can get a better impl, or set the CS delay to 0 if they've manually verified their chip is slow enough to not violate timings
<adamgreig[m]>
and on most platforms you would get the ns-level delays...
<adamgreig[m]>
(I appreciate on most platforms you won't get single-ns resolution, but 10 or 20ns resolution is plausible)
<therealprof[m]>
Sorry, gotta run. :wave:
<therealprof[m]>
s/:wave:/👋/
<therealprof[m]>
dirbaio[m]: Or not share the bus?
<jannic[m]>
Yes, for CS delay it looks useful. Perhaps make the warning that it might round up a little bit more visible, so people don't start using it in timing loops.
<adamgreig[m]>
so the suggestion is 1) rename DelayUs to BasicDelay/SimpleDelay/Delay, add delay_ns 2) add Operation::DelayNs to SpiDevice
<dirbaio[m]>
therealprof[m]: you still need CS delays even if you don't share
<adamgreig[m]>
but if you're not sharing you can just use the SpiBus, OutputPin, and Delay
<adamgreig[m]>
(e.g. on Linux you can Delay by a few ns but you can't put a few-ns delay into an SPI transaction)
<dirbaio[m]>
but a driver will likely use SpiDevice, to support sharing if needed
<adamgreig[m]>
can't say this is the conclusion I foresaw reaching but korken89 might be pleased :P
<dirbaio[m]>
3) document you're not supposed to use Operation::Delay for CS delays
<dirbaio[m]>
4) add something to e-h-b
<adamgreig[m]>
we could just call the trait DelayNs
CanuteTheGreat has joined #rust-embedded
<adamgreig[m]>
add something to e-h-b to support delays around CS using the new Delay trait?
<dirbaio[m]>
yeah?
<dirbaio[m]>
maybe later
<adamgreig[m]>
and document that you shouldn't use delay for cs delays, but I guess we can't stop you and people might anyway and it'l work at least, even if we don't like it
<dirbaio[m]>
but at least document that's the direction we'd like to go
<adamgreig[m]>
yea
<adamgreig[m]>
drivers can update later easily
<adamgreig[m]>
so I wouldn't be mad if they used operation::delay for now and updated once there was some other good answer
<bartmassey[m]>
👋
<adamgreig[m]>
I think I prefer DelayNs over SimpleDelay or BasicDelay, by the same logic we used to reach the current DelayUs
<adamgreig[m]>
also it's better because capital µ is M but DelayMs would be very confusing
<dirbaio[m]>
DelayMsNsUs
<dirbaio[m]>
DelayXs
<mameluc[m]>
`Delay<U>`
<dirbaio[m]>
nooooo no typestates
<adamgreig[m]>
I'll open an issue for it
<adamgreig[m]>
that's all we have time for today, thanks everyone!
<adamgreig[m]>
e-h 1.0 draws ever closer 🕚️
CanuteTheGreat has left #rust-embedded [Leaving]
<jannic[m]>
How many rc releases can we do during the one month left?
<adamgreig[m]>
is that a challenge :P
<adamgreig[m]>
ah sorry bartmassey, didn't see your thread reply earlier
<adamgreig[m]>
the problem is we don't expect to ever want to use core Duration, not only because it requires more memory, but because all arithmetic on it takes longer too as a consequence, but we don't yet know what we will want, so we're punting on that for now
<adamgreig[m]>
you can't let the user choose because the "users" aren't the ones writing the code that uses e-h, generally speaking
<adamgreig[m]>
if you're writing a driver, you choose what e-h trait you use (like Delay), and use it, and then the person writing an application wants to use your driver, but they're the one who's picked a platform that is big/small/fast/slow and they don't get a choice about what you used in your driver
<dirbaio[m]>
yeah
<dirbaio[m]>
embedded-hal 0.2 had 2 delay traits (DelayMs, DelayUs...)
<dirbaio[m]>
* also, re "why not have 2 traits and let the user choose": having multiple traits has a cost
<dirbaio[m]>
embedded-hal 0.2 had 2 delay traits (DelayMs, DelayUs)
<dirbaio[m]>
but they were generic, effectively turning them into N traits
<dirbaio[m]>
if your HAL implemented `DelayUs<u32>` and your driver needed `DelayMs<u16>` then you were screwed
<danielb[m]>
why do separate DelayXs traits exist btw?
<dirbaio[m]>
rationale was each HAL could implement only what the hardware supports
<dirbaio[m]>
so you'd be guaranteed at compile time to not accidentally use something the hardware can't do
<dirbaio[m]>
but now we know that causes annoying fragmentation, fewer traits are better because it increases the chance any HAL can work with any driver
<dirbaio[m]>
especially in the case of delays, where if the hardware doesn't support ns-resolution delays you can just round up
<mabez[m]>
<dirbaio[m]> "last-call for objections to..." <- No objections here, the 1.48 release was over 3 years ago! They should be updating their msrv, not trying to reduce ours
<danielb[m]>
(as my comment is on the issue) no objections
<dirbaio[m]>
I don't know, iirc one of the goals of the Duration thing was it'd specify the duration in units of the hardware's exact ticks
<dirbaio[m]>
to avoid rounding
<dirbaio[m]>
if you convert Duration -> ns -> Duration you're rounding twice
<adamgreig[m]>
why would you convert twice, because the hal implements delay_ns in terms of its own duration?
<adamgreig[m]>
the hal can just provide its own impl for delay(duration) when it's out?
<dirbaio[m]>
yeah
<dirbaio[m]>
but it feels "wrong" to have it be "bad by default"
<adamgreig[m]>
it's just "backwards compatible by default"
<dirbaio[m]>
if anything, i'd say delay_[mun]s should be defaulted on top of delay(Duration)
<adamgreig[m]>
otherwise we'll have BasicDelay and Delay traits, and drivers will use one and hals will want to provide the other and all the usual annoyance of two overlapping traits
<adamgreig[m]>
that would be nice but breaking
<dirbaio[m]>
we can do it backwards-compatibly if we add `trait Delay { fn delay(Duration) }` and `impl <T: Delay> BasicDelay for T`
<dirbaio[m]>
I think...?
<adamgreig[m]>
hm, I feel like we've hit unexpected problems with providing impls like the latter before
<dirbaio[m]>
yeah... it might conflict with `impl <T: BasicDelay> BasicDelay for &mut T`
<adamgreig[m]>
if we add delay(Duration) to the existing Delay trait, we just document that implementors should prefer to implement delay() and then do delay_ns() in terms of it, it wouldn't be a big deal for most implementors?
<adamgreig[m]>
and still work OK for all users and be backcompat
<adamgreig[m]>
or maybe we never find a Duration everyone likes
<dirbaio[m]>
lol
<GrantM11235[m]>
If there is a default impl for delay(Duration), that means there would need to be a default timebase, right?
<dirbaio[m]>
GrantM11235[m]: `Duration` might be an associated type, so the impl can choose the timebase
<adamgreig[m]>
just mean that Duration must be convertible to ns
<dirbaio[m]>
something like fugit
<dirbaio[m]>
just not completely unconstrained like in the 0.2 traits
<dirbaio[m]>
so users can still do math with it and convert it to/from ns/ms/us/s
<adamgreig[m]>
the fugit types are pretty nice imo
<adamgreig[m]>
the embassy ones are convenient when you have a compile-time-globally-configured timebase of course
firefrommoonligh has quit [Quit: Idle timeout reached: 172800s]
<dirbaio[m]>
wholesale adopting fugit is definitely an option
<dirbaio[m]>
it's a bit complex for my taste
<dirbaio[m]>
OTOH I agree the embassy solution is not flexible enough for e-h
<dirbaio[m]>
so iduk
<dirbaio[m]>
* so idk
<adamgreig[m]>
maybe we can end up with a duration trait that both embassy and fugit could impl, idk
<dirbaio[m]>
traits all the way down
<adamgreig[m]>
always was
<dirbaio[m]>
we're getting sidetracked tho
<adamgreig[m]>
anyway, Delay?
<adamgreig[m]>
if we already know what Duration looks like then for sure we just have delay_muns in terms of delay(duration)
<adamgreig[m]>
but since we don't and won't, but we can later add it in terms of delay_ns and just suggest people don't use the default impl...?
<adamgreig[m]>
adamgreig[m]: and we wouldn't want a BasicDelay trait in this situation, just more methods on Delay
<dirbaio[m]>
i'm not sure
<adamgreig[m]>
so either we end up one day with Delay and BasicDelay, the latter essentially deprecated since Delay can have delay_muns
<adamgreig[m]>
or we end up one day with just Delay, but it has delay() in terms of delay_ns for historic reasons
<dirbaio[m]>
we reached the consensus a while ago to reserve the "good" Duration name for the trait with the "good" Duration
<dirbaio[m]>
this is walking back on that
<adamgreig[m]>
the good Delay name*?
<dirbaio[m]>
yes sorry
<adamgreig[m]>
yea
<dirbaio[m]>
s/Duration/`Delay`/, s//`/, s//`/
<adamgreig[m]>
well I just wonder if it turns out having u32 range for m/u/ns is basically enough for everyone anyway, and we can add the good delay(Duration) in a back-compat way later with fairly minimal long-term pain
<dirbaio[m]>
so if we're doing that, we should have a proper discussion in the weekly meeting with the rest of the HAL team maybe
<adamgreig[m]>
vs keeping the BasicDelay (it's basic because we don't know what Delay will be yet, not because it's less featureful?) forever alongside the good Delay
<dirbaio[m]>
vs if we do BasicDelay now we're not closing any doors
<adamgreig[m]>
yea, perhaps even a little rfc with voting in the repo
<dirbaio[m]>
we can either add trait Delay or add a fn delay(Duration) to BasicDelay
<dirbaio[m]>
* we can either add trait Delay or add a fn delay(Duration) to BasicDelay later
<dirbaio[m]>
when we know what Duration looks like
<adamgreig[m]>
but then we have a vestigal BasicDelay plus the real Delay forevermore
<dirbaio[m]>
that was the original plan, yes
<dirbaio[m]>
have a vestigial DelayUs
<adamgreig[m]>
vs the only(?) downside of using Delay now is that eventually we have a slightly backwards default impl in the end
<dirbaio[m]>
yeah...
<dirbaio[m]>
iirc the possibility of adding fn delay() to the current trait wasn't discussed
<dirbaio[m]>
nobody thought of it
<mabez[m]>
adamgreig[m]: but we can mark it as deprecated, without violating semver, right?
<dirbaio[m]>
it definitely changes things
<adamgreig[m]>
but in the meantime we get the short simple obvious trait name, and it already lets you do ns/us/ms delays... so
<jannic[m]>
To me the name BasicDelay looks confusing. Everybody will ask "and whats the full-featured delay?" That's wasn't an issue with DelayUs.
<mabez[m]>
Even if we never remove it, we can still point users to the new trait if that's the route we want to go down
<adamgreig[m]>
mabez: yea, but we still have to _keep_ it, and drivers and HALs will keep using and implementing it
<adamgreig[m]>
bit mean to mark it deprecated if we never plan to remove it maybe, since you'll get warnings forever lol
<dirbaio[m]>
jannic[m]: people will ask "where's the millisecond delay?" 🙃
<dirbaio[m]>
ah no
<dirbaio[m]>
there's one detail
<adamgreig[m]>
jannic[m]: yea. I understand the arguments against DelayNs, but I agree that BasicDelay implies another Delay that just doesn't exist, or that this delay is somehow... basic
<dirbaio[m]>
if Duration is an associated type, we can't add it to Delay backwards-compatibly
<adamgreig[m]>
aw, there's always one detail
<danielb[m]>
dirbaio[m]: and here I am just looking for AcidicDelay 🙈
<dirbaio[m]>
and it needs to be an associated type if we want it to match the hardware tick rate
<dirbaio[m]>
let's not bank our future on unstable features 🥲
<adamgreig[m]>
the issue's been open longer than the wg, lol
<dirbaio[m]>
been there done that
<dirbaio[m]>
so
<adamgreig[m]>
we could use fugit types of a generic timebase without an associated type, right?
<dirbaio[m]>
you need the trait to fix the timebase somehow
<dirbaio[m]>
either associated type, or associated num/denom consts
<dirbaio[m]>
if you let the user pick, they'll pick anything (like, microseconds!) and now you're back to rounding
<adamgreig[m]>
oh, so the HAL could constrain it if it was an associated type?
<dirbaio[m]>
there's only a benefit if the driver can "know" what the hardware timebase is so it can do all its math in it
<adamgreig[m]>
if the driver constrains it it seems a bit pointless
<dirbaio[m]>
yep
<dirbaio[m]>
anyway
<dirbaio[m]>
proposal
<dirbaio[m]>
release rc2 with BasicDelay, discuss whether to rename to Delay in the next weekly meeting?
<dirbaio[m]>
(though IMO due to the associated type thing, we shouldn't rename it to Delay)
<dirbaio[m]>
or release rc2 with DelayNs
<dirbaio[m]>
I don't mind which one
<dirbaio[m]>
but I wouldn't rename to Delay now
<adamgreig[m]>
if we're hopefully going to rename it anyway I'd rather DelayNs now, it's basically the same as what we have already
<adamgreig[m]>
and then yea, let's discuss maybe calling it Delay next week
<dirbaio[m]>
I don't think we're going to rename it to Delay
<dirbaio[m]>
unless we decide to give up on the magic Duration
<dirbaio[m]>
* unless we decide to officially give up on the magic Duration
<adamgreig[m]>
we'd only be giving up on a particular form of it
<adamgreig[m]>
still lots of possibly-useful Durations that wouldn't require an associated type
<dirbaio[m]>
I think the most useful Duration does require an associated type
<adamgreig[m]>
I don't quite follow why if it's the driver that gets to set the timebase rather than the hal
<adamgreig[m]>
but yea, let's discuss that separately, it doesn't need to block rc2
<dirbaio[m]>
👍️
<dirbaio[m]>
well "driver chooses the timebase" would work if the HAL can change the hardware to match it, which is not always true
<dirbaio[m]>
or if the HAL does rounding to convert from driver timebase to hardware timebase
<dirbaio[m]>
* (or if, * hardware timebase, but then we haven't accomplished anything)
<adamgreig[m]>
my guess is the driver always wants to specify delays in terms of time units, not ticks, so it's just that if the duration type knows the timebase then the compiler can do the conversion for us
<adamgreig[m]>
probably 99% of drivers will be using the delay_nums methods
<dirbaio[m]>
yeah...
<danielb[m]>
the HALs could provide the duration type along with the Delay impl, couldn't they?
<adamgreig[m]>
easier, simpler, does what they actually need, no messing with types or timebases?
<dirbaio[m]>
i'm starting to think ms/us/ns covers all use cases and is stupid simple
<adamgreig[m]>
the advantage is just that if the timebase is statically known then the compiler can do the conversion for us, but the hal could already make that the case with its delay impl, I think
<dirbaio[m]>
for the other timer/capture/etc traits it's less clear tho
<adamgreig[m]>
eg in embassy the delay impl can be compile-time constant conversion and inlineable
<adamgreig[m]>
those ones want frequencies which might be more or less than 1, which is much trickier
<adamgreig[m]>
stupid reciprocals
<adamgreig[m]>
dirbaio[m]: I think it covers all _use cases_, but not necessarily at maximum efficiency/configurability, in terms of possibly needing runtime conversion calculations, and always being a u32
<dirbaio[m]>
renamed to DelayNs
<adamgreig[m]>
so your AVR is still going to be a bit annoyed at being told a u32 instead of a u8, but at least it's not a u64 lol
<adamgreig[m]>
the dream Duration is probably configurable over size as well as having a static timebase (ie fugit...), but I don't think that changes use cases, from the driver pov at least
<adamgreig[m]>
how long until we just use core::time::Duration lol
<dirbaio[m]>
AVR 💀
<dirbaio[m]>
IMO core::time::Duration is just too big
<dirbaio[m]>
96 bytes (128 due to alignment?)
<adamgreig[m]>
2039: having survived the epocolypse, the embattled embedded-wg finally add delay_ps()
<adamgreig[m]>
microcontrollers are gonna be so fast by then, right :D
<vollbrecht[m]>
after 2038 32 bit overflow of time?
<dirbaio[m]>
terahertz MCUs!
<dirbaio[m]>
and 70 versions of the stm32 usart peripheral by then
<dirbaio[m]>
and 459 ADCs
<dirbaio[m]>
* and 459 ADC versions
<adamgreig[m]>
what datatype do you use to store the version field in embassy 😆
<adamgreig[m]>
I imagine the "dirbaio is typing" disappearing is you checking it's not a u8 in a cold sweat
<dirbaio[m]>
it's a string
<adamgreig[m]>
should be safe for a while then
<dirbaio[m]>
not forever though, this is ST we're dealing with
<adamgreig[m]>
stm64 when
<adamgreig[m]>
they can start afresh
<danielb[m]>
knowing ST they'll come up with STM49 first probably
<adamgreig[m]>
oh nooo
<adamgreig[m]>
what's the 49th bit for
<dirbaio[m]>
the "naughty/nice" bit
<danielb[m]>
the "are we using a different calling convention right now" bit
<adamgreig[m]>
"read always naughty"
<adamgreig[m]>
RAN/WI
<dirbaio[m]>
"is MCU on" bit. reads 1 when on, undefined when off