crabbedhaloablut has quit [Ping timeout: 240 seconds]
crabbedhaloablut has joined #rust-embedded
<re_irc>
<marmrt> I'm having a problem with atsamd-hal where transmitting over a uart interface with an rts pin is not working.
<re_irc>
I've managed to isolate the error: the rts-pin is hardware controlled and goes high whenever the receive buffer is full or the receiver is disabled. uart.write() does not disable the receiver and the receive buffer never gets filled
<re_irc>
<marmrt> Is there a way to enable/disable the receiver from the hal?
<re_irc>
<marmrt> Maybe this is just me using hardware in a unexpected way.
<re_irc>
<marmrt> The setup is MCU->RS485 tranceiver-> Laptop.
<re_irc>
The RTS pin on the mcu is connected to both the Receive Enable (active low) and Driver Enable (active high) pins on the rs485 tranceiver.
<re_irc>
So, like the real issue is that the tranceiver is half duplex and the mcu is full duplex
gpanders_ has joined #rust-embedded
<re_irc>
<James Munns> FWIW, in my RS485 stuff, I use a separate GPIO pin to manage the receive/send enable pin(s)
<re_irc>
<James Munns> Not super sure about atsamd-hal, but other chips often let you create the UART port without a "hardware" cts/rts pin, so you can make a small wrapper that does the right thing with the GPIO(s) you are using
<re_irc>
<adamgreig> room meeting time again! agenda is https://hackmd.io/ZgvnlslqTzm72aGP7PyxOw, please add anything you'd like to announce or discuss and we'll start in 5 min :)
<re_irc>
<adamgreig> ok, let's start!
<re_irc>
<adamgreig> couple of release announcements: cargo-binutils 0.3.5 has a small update to let you know if you've forgotten to "rustup component add llvm-tools-preview"
<re_irc>
<adamgreig> embedded-hal 0.2.7 released with the new CAN traits
<re_irc>
<adamgreig> embedded-hal 1.0.0-alpha.7 released with the latest hot breaking changes
<re_irc>
<adamgreig> that's all my announcements, anyone else have anything?
<re_irc>
<adamgreig> 8 days until rust 1.59 and stable asm 🔥
<re_irc>
<firefrommoonlight> Rust on embedded owns - that's all
<re_irc>
minor downsides for async usage (mostly no async traits).
<re_irc>
no downsides for blocking usage (yes, you can use embassy-nrf, embassy-stm32 without async! they have some interesting advantages, such as a unified API for all stm32 families)
<re_irc>
<adamgreig> nice!
<re_irc>
<adamgreig> in resources news, embedonomicon CI is fixed so a few pending PRs got merged, but no update on tidying up the awesome-embedded-rust list
<re_irc>
<adamgreig> likewise nothing on the cortex-m alpha or use of critical section... but i'll carry that over for next week
<re_irc>
<adamgreig> in embedded-hal, all the traits with unconstrained associated types (like a generic Time type) have been removed from the 1.0 branch, eldruin linked to the roadmap for tracking adding them back: https://github.com/rust-embedded/embedded-hal/issues/357
<re_irc>
<adamgreig> and dirbaio updated his proposal for the SPI trait, which splits it into a separate bus/device and incorporates transactions and control of CS: https://github.com/rust-embedded/embedded-hal/pull/351
<re_irc>
<eldruin> the biggest issue is managed CS
<re_irc>
<almindor> dirbaio: does the removal of the "ManagedCS" mean that as it is now a user cannot specify if the SPI device they got manages the CS?
<re_irc>
<adamgreig> might be worth editing the description to put all the closed/resolved issues at the bottom and leave just the open issues/PRs at the top?
<re_irc>
<eldruin> adamgreig: sure, I did it the other way around :D
<re_irc>
<adamgreig> aha, that's fine too :D
<re_irc>
<adamgreig> oh!
<re_irc>
<adamgreig> maybe just put a line break :P
<re_irc>
<eldruin> but maybe some separation would be nice
<re_irc>
<eldruin> exactlu
<re_irc>
<eldruin> * exactly
<re_irc>
<adamgreig> I got confused by some of them bring struck through and some being merged/closed
<re_irc>
<eldruin> yeah, it does not look very good
<re_irc>
<eldruin> actually it is just a repetition of the 1.0.0 milestone
<re_irc>
<adamgreig> I think it's fine with just a break between complete/to-do items
<re_irc>
<eldruin> it's done now
<re_irc>
<adamgreig> perfect, thanks
<re_irc>
<dirbaio> almindor: "user" as in the driver? kinda yes, but arguably SplDevice already strongly implies there's a CS. a SplDevice impl on a shared bus without CS makes no sense.
<re_irc>
<dirbaio> * CS, because
<re_irc>
<dirbaio> I'm still not 100% sure on that (whether to have a ManagedCS trait or not)
<re_irc>
<almindor> dirbaio: So the implication is, if not using the whole Bus the CS is controlled one way or another (hw or built-in sw in the driver implementation)
<re_irc>
<dirbaio> the way I see it is: SpiDevice represents "a way to talk to one particular SPI device"
<re_irc>
<dirbaio> so a shared-bus SpiDevice HAS to have a CS, it's garbage otherwise
<re_irc>
<dirbaio> and an exclusive-bus SpiDevice may or may not have it
<re_irc>
<dirbaio> and it's the user's responsibility to ensure the SpiDevice is configured OK
<re_irc>
<adamgreig> one weird spi use case i have that can be annoying with CS is a device where you assert CS and then stream in a few hundred kB of data in one go, then deassert CS, but in my application the data arrives in chunks over some other interface (network, uart, whatever)
<re_irc>
<almindor> yes, but say you're writing a device driver that needs to know the CS situation (if it's unmanaged it needs to manage it itself), how would that work?
<re_irc>
<adamgreig> so ideally i'd set CS low, then occasionally do some transfers, then much later set CS high again
<re_irc>
<dirbaio> No, drivers never manage CS on their own, they assume the SpiDevice will do it
<re_irc>
<adamgreig> and to reset, need to assert CS, pulse reset pin, wait a little bit, then deassert CS, without actually sending anything... then send 8 clocks with CS high, then assert CS again to do data
<re_irc>
<dirbaio> but we still want to allow SpiDevice's that don't manage CS
<re_irc>
<dirbaio> for example if the user ties CS low in the PCB
<re_irc>
<adamgreig> all of which is fairly easy if there's an option to have the driver control its own CS pin when required, even if it's just that that driver takes an OutputPin or whatever I guess
<re_irc>
<eldruin> adamgreig: could you add your use case somewhere in the issues? it seems interesting
<re_irc>
<Oddstr13> you would want a guarantee that other devices on the bus are de-asserted to avoid junk on the SPI bus, but you also want to be able to talk to devices wanting the CS to stay asserted for multiple transactions
<re_irc>
<adamgreig> but would HAL implementations of SpiDeviceBase require a CS be passed in?
<re_irc>
<adamgreig> eldruin: sure, maybe on 351?
<re_irc>
<eldruin> adamgreig: sure, it's a bit all over the place
<re_irc>
<dirbaio> adamgreig: so you want a way to, for example, "store" an "in-progress" transaction in "self"?
<re_irc>
<adamgreig> dirbaio: I guess by permitting "SpiDevice that doesn't manage CS because it's tied low on the PCB" you also permit "SpiDevice that does entirely manage CS by being given a separate OutputPin"?
<re_irc>
<adamgreig> dirbaio: I think I just want to be able to set CS high/low in the driver, and otherwise use the normal write traits
<re_irc>
<Oddstr13> I can also think of a scenario where you might want to send the same command to multiple devices, but I don't have any specific examples. Something like DS18B20's all devices start measurement
<re_irc>
<adamgreig> hm, maybe I misunderstood your example impl
<re_irc>
<adamgreig> in your SpiDeviceWithCs, you can't construct it without giving it a CS pin, and having done so, the driver can't use CS, it's just asserted during transactions, right?
<re_irc>
<dirbaio> adamgreig: If driver starts a transaction and then returns, then another driver wants to use the bus, how do you handle that?
<re_irc>
<adamgreig> but presumably SpiDeviceWithCs is provided by the HAL
<re_irc>
<adamgreig> dirbaio: in my case there's no sharing of this bus (not least because I leave CS asserted for maybe ~seconds while data comes in)
<re_irc>
<almindor> there's also a messy combination of "manual sharing" of CS-aware and CS-less devices on the same buss
<re_irc>
<almindor> * bus
<re_irc>
<dirbaio> adamgreig: in that case the driver could own the entire SpiBus, and the CS pin, and manage everything itself as usual..?
<re_irc>
<adamgreig> hmm, yea, I think that would work fine
<re_irc>
<dirbaio> ie
<re_irc>
<dirbaio> if you want your driver to NOT allow sharing, then make it own the entire SpiBus
<re_irc>
<dirbaio> if you do want to allow sharing, then own just a SpiDevice
<re_irc>
<adamgreig> got it
<re_irc>
<dirbaio> +but then you're bound by the slightly-more-restricted SpiDevice API
<re_irc>
<adamgreig> and if you own the SpiDevice, you assume the bus will somehow handle CS for you
<re_irc>
<adamgreig> but if you take the whole SpiBus, you do the CS yourself
<re_irc>
<dirbaio> yep
<re_irc>
<adamgreig> and with the SpiDevice API, you don't have any choice but to let the bus handle CS, because you just do transactions?
<re_irc>
<dirbaio> yep
<re_irc>
<adamgreig> so most "normal" SPI devices would happily implement SpiDevice (or take something that implements SpiDevice?), and wthe occasional annoying thing can take an SpiBus instead?
<re_irc>
<almindor> I'm guessing we don't want to abstract over CS control settings?
<re_irc>
<adamgreig> like timing and polarity?
<re_irc>
<dirbaio> drivers that use SPI for "cursed" stuff would also own SpiBus, not SpiDevice.
<re_irc>
examples: WS2812B waveform generation with SPI, using an SPI as a logic analyzer...
<re_irc>
<almindor> yes
<re_irc>
<adamgreig> dirbaio: that certainly sounds like it would work fine for all my weird SPI uses
<re_irc>
<almindor> this would limit "no-CS" devices to be exclusive
<re_irc>
<adamgreig> if anything uses CS with positive polarity it should be thrown in the bin lol 🚮
<re_irc>
<dirbaio> CS settings could be handled by the "bus mutexing" crate
<re_irc>
<dirbaio> almindor: how do you even use a CS-less chip in a shared bus?
<re_irc>
<adamgreig> yea
<re_irc>
<almindor> dirbaio: hacking it in as a separate receiver :D it works with e.g. ST7789 (no-cs variant) hooked to the same lines as another compatible display. It's a hack tho I don't think it's a logical abstraction blocker
<re_irc>
<eldruin> dirbaio: I think the point would be manual-CS devices. you cannot share the bus among several manual CS devices
<re_irc>
<almindor> yes that too
<re_irc>
<dirbaio> ooof what
<re_irc>
<Oddstr13> dirbaio: only way I can think of would be to implement CS yourself, I know some transistor/mosfet magic is done with some devices that doesn't let go of the MISO pin when not selected
<re_irc>
<dirbaio> you mean, if drivers want to manually manage CS _and_ still allow sharing?
<re_irc>
<eldruin> although, you could share with several manual CS devices by just taking spibus and using shared-bus, just like now
<re_irc>
<almindor> yes
<re_irc>
<almindor> imagine a MCU that has crappy HW CS setup
<re_irc>
<eldruin> so we can also go full circle :D
<re_irc>
<almindor> say it doesn't support timing you need or such
<re_irc>
<eldruin> sometimes there are devices that have special timing requirements WRT CS assertion/deassertion and actual data transfer
<re_irc>
<eldruin> I think you will need manual CS for those
<re_irc>
<adamgreig> if the MCU is crappy, the HAL needs to provide a suitably useful SpiBus with CS control
<re_irc>
<eldruin> * those, but still might want to share
<re_irc>
<dirbaio> we could do something like this instead
<re_irc>
<almindor> yes, I had the pleasure of working with such. Sometimes your HW CS control settings support what you need, but if they don't then what?
<re_irc>
<dirbaio> this is SUPER flexible, would allow all cursed usecases, like adamgreig 's
<re_irc>
<dirbaio> but it's super prone to misuse
<re_irc>
<adamgreig> yea, way more chance for annoynig runtime errors
<re_irc>
<adamgreig> and you can't really do anything to handle it
<re_irc>
<almindor> we can't make those unsafe can we?
<re_irc>
<eldruin> yeah, would be more of a footgun
<re_irc>
<adamgreig> it's not really a memory safety concern anyway...
<re_irc>
<adamgreig> I feel like the current suggestion solves 95% of SPI devices, and the rare stragglers can take the SpiBus themselves, and shared-bus can still exist to let you share _that_ manually carefully
<re_irc>
<GrantM11235> eldruin: You can put delays in the closure
<re_irc>
<eldruin> I think the solution is fine
<re_irc>
<eldruin> adamgreig: exactly
<cr1901>
>and shared-bus can still exist to let you share _that_ manually carefully
<re_irc>
<almindor> yeah I tend to agree
<cr1901>
Providing an escape hatch sounds ideal
<re_irc>
<dirbaio> how does "shared-bus" allow safe sharing with manual CS?
<re_irc>
<almindor> I'm a bit "warped" in my experience because I combined some crazy things in a hacky way so I see those funky hack-cases
<re_irc>
<dirbaio> I think it doesn't
<re_irc>
<dirbaio> if a driver "starts" a transaction then returns without ending it
<re_irc>
<eldruin> just like now
<re_irc>
<dirbaio> other drivers can still go and read/write to the bus
<re_irc>
<almindor> shared-bus could be shared via a mutex or such?
<re_irc>
<eldruin> jsut gives you several spi bus instances
<re_irc>
<eldruin> jsut checked at runtime that you do not use while using the other
<re_irc>
<eldruin> CS-wise you are on your own, though. but you asked for it so
<re_irc>
<dirbaio> > assumes that drivers will only activate their CS right before communicating and deactivate it right afterwards and that there is no way for any other code in the same thread to do anything else in that time
<re_irc>
<dirbaio> and lock the bus for the entire tranasaction....
<re_irc>
<eldruin> so.. transactional SPI? :D
<re_irc>
<almindor> I think we should just make it scream in the docs for those edge cases. Trying to compile-time abstract-protect every use case is overkill
<re_irc>
<almindor> it'd just make the traits extremely complicated resulting in nobody using them
<re_irc>
<dirbaio> eldruin: or a .transaction() method that gives you the bus :D
<re_irc>
<dirbaio> +so you can do multiple read/writes
<re_irc>
<eldruin> dirbaio: interesting
<re_irc>
<Oddstr13> dirbaio: sounds like what you'd want is something like this, which locks the bus on start, and unlocks on end transaction
<re_irc>
<adamgreig> so that an SpiDevice could 'take' the whole SpiBus for a bit, and put it back later? what happens if another device tries to do a transaction while the bus is taken?
<re_irc>
<adamgreig> I guess same thing that happens if you take the bus while it's taken, i.e. an error
<re_irc>
<Oddstr13> BusBusyError? :P
<re_irc>
<adamgreig> sounds like it might make life a fair bit harder for HALs?
<re_irc>
<dirbaio> I was joking here, this is exactly what my PR does
<re_irc>
<almindor> that's how we implemented it in e310-hal we made the bus "borrowed" each time a device needed to do anything
<re_irc>
<adamgreig> yea, but borrowing it for a while is different to taking it and putting it back later
<re_irc>
<almindor> the only diff is the locking enforcement AFAICS
<re_irc>
<eldruin> dirbaio: sorry, I forgot how it looked exactly
dsmith-work has left #rust-embedded [ERC (IRC client for Emacs 27.1)]
<re_irc>
<dirbaio> btw there's also async
<re_irc>
<dirbaio> with async you could start an SPI transaction, and _within_ the SPI transaction, await receiving bytes from somewhere else and stream them to SPI
<re_irc>
<adamgreig> so the bus would be borrowed across the await?
<re_irc>
<dirbaio> adamgreig: which solves this nicely ☝️
<re_irc>
<dirbaio> yes, the bus would be locked during the awaits
<re_irc>
<dirbaio> which works nicely if the SpiDevice impl uses an async mutex internally
<re_irc>
<dirbaio> so tasks trying to use the same bus would wait for the other to finish
<re_irc>
<almindor> there's no way around that, unless you're thinking about splitting your transmissions into small chunks and scheduling them somehow
<re_irc>
<eldruin> uhh, it's probably difficult to find out who locked the bus if something goes wrong
<re_irc>
<dirbaio> well that's true everytime you use a mutex 🤷
<re_irc>
<eldruin> but yeah, probably there is no way around that
<re_irc>
<adamgreig> dirbaio: would that need an async version of this trait?
<re_irc>
<dirbaio> in the end this thing is a mutex, with all the caveats
<re_irc>
I was planning on trying that out next, but I can't see why wouldn't it work
<re_irc>
<adamgreig> nice
<re_irc>
<adamgreig> second Q, would you do the same for i2c :p
<re_irc>
<dirbaio> hahaha
<re_irc>
<dirbaio> the million dollar question
<re_irc>
<dirbaio> maybe!!
<re_irc>
<adamgreig> in some ways it's even easier because no CS
<re_irc>
<almindor> isn't i2c easier?
<re_irc>
<eldruin> yeah with i2c shared-bus is fine, but this mechanism is pretty nice so it might be interesting to do the same for all protocols
<re_irc>
<almindor> oh definitely!
<re_irc>
<dirbaio> what about addresses?
<re_irc>
<almindor> any "bus" should probably use this principle
<re_irc>
<almindor> address would need to be on the device no?
<re_irc>
<dirbaio> does an SPI transaction have to be all against the same address?
<re_irc>
<dirbaio> * I2c
<re_irc>
<eldruin> (I have not looked about the new version of the proposal yet, though)
<re_irc>
<GrantM11235> One of the things I like about your proposal is that it gives all spi devices a transaction closure method. That could allow increased performance even for devices that don't have a CS
<re_irc>
<eldruin> dirbaio: no
<re_irc>
<dirbaio> I think you can do a repeated-start then talk to a different addr
<re_irc>
<dirbaio> oof
<re_irc>
<dirbaio> wtf
<re_irc>
<eldruin> uh, sorry I misunderstood you
<re_irc>
<eldruin> I am not sure about repeated start. I have never seen a repeated start and then a different addresss
<re_irc>
<dirbaio> so, for example, we could do this:
<re_irc>
<eldruin> you would need to specify when you want a repeated start, though, like maybe you put two reads after another but do not want a repeated start in between
<re_irc>
<adamgreig> when the answer paragraph 3/8 starts "As a matter of fact there was work I did, some years back, designing some slave devices into FPGA type parts."
<re_irc>
<adamgreig> eldruin: that would just be two transactions, right?
<re_irc>
<eldruin> I'm getting confused xD
<re_irc>
<adamgreig> hah, me too, I feel like I derailed this a bit with the i2c grenade
<re_irc>
<eldruin> I would need to think about it once we have a proposal
<re_irc>
<adamgreig> yea
<re_irc>
<eldruin> but I think it is easier than SPI
<re_irc>
<adamgreig> I think it would not be unreasonable to insist on same address per transaction if that simplifies the api nicely
<re_irc>
<dirbaio> "1" and "2" would be same transaction
<re_irc>
<adamgreig> i.e. you have to restart to change between read/write
<re_irc>
<dirbaio> Yes, I mean for "write + write"
<re_irc>
<adamgreig> yea
<re_irc>
<eldruin> you can always do the whole thing yourself like with manual cs and spibus as we talked earlier so we can do the nice thing for 95% of uses and whoever does something special can have it his way too
<re_irc>
<dirbaio> so we need some way to distinguish 1 and 2
<re_irc>
<adamgreig> yea, hm
<re_irc>
<dirbaio> btw, the current Transactional API as specified will do "1", there's no way to do "2" AFAICT
<re_irc>
<adamgreig> I was gonna say if anything maybe 2 is more useful but I guess a lot of the time you have a "command buffer" and a "data buffer" and you want to stream both at once or something
<re_irc>
<dirbaio> adamgreig: yep, this is super common in my experience
<re_irc>
<dirbaio> "2" is less common
<re_irc>
<eldruin> the question would be why do you need a restart. mostly it is just to change the mode from write to read, so a restart between two writes does not seem very useful
<re_irc>
<dirbaio> maybe there's some device out there that requires it
<re_irc>
<dirbaio> * it? I dunno
<re_irc>
<eldruin> sure, but that would be the 5% case where you can just bet the whole bus and use shared-bus
<re_irc>
<dirbaio> um
<re_irc>
<dirbaio> if we did this, I would remove i2c Transactional
<re_irc>
<eldruin> yeah, there would not be much reason for it
<re_irc>
<eldruin> that would be fine with me
<re_irc>
<dirbaio> and then there will no longer be a way to do a read-read restart, or a write-write restart
<re_irc>
<dirbaio> even if you own the entire bus
<re_irc>
<eldruin> yeah we still need to check all those edge cases
<re_irc>
<adamgreig> the restarts are useful on multimaster buses especially
<re_irc>
<adamgreig> because the hold the bus for the duration, no one else can start talking in between
<re_irc>
<adamgreig> * they
<re_irc>
<dirbaio> maybe you want to send multiple commands, each command requires a restart, and you don't want another master to steal it from you in between
<re_irc>
<dirbaio> yeah
<re_irc>
<adamgreig> I wouldn't be surprised if some devices require restart instead of stop+start to do a full read too
<re_irc>
<adamgreig> often you start, addr+W, write register address, restart, addr+R, read register contents, stop
<re_irc>
<eldruin> they do
<re_irc>
<eldruin> VEML6030 does
<re_irc>
<dirbaio> yep, usually if you do a full stop, the device forgets the register addr
<re_irc>
<eldruin> that was the reason why I added transactional to l-e-h. somehow every other device until then was fine with doing stop+start and not a real restart
<re_irc>
<adamgreig> but I guess write-restart-write or read-restart-read is pretty uncommon
<re_irc>
<eldruin> I have never seen it, but I do not know if some device needs it
<re_irc>
<adamgreig> and restart automatically if you do write() then read()?
<re_irc>
<eldruin> hmm, but if you do not do automatic restarts, if you forget about adding a restart, you will continue writing with an empty read buffer
<re_irc>
<adamgreig> or if you change address between calls to write()?
<re_irc>
<dirbaio> by default, read/write do restarts only if needed (addr changes, direction changes)
<re_irc>
and you force a write-write or read-read restart with .restart()
<re_irc>
<adamgreig> so successive read()s or write()s with the same address would be merged, but if you change r->w or w->r or change address, it restarts, and if you want to restart between multiple write()s with same address, you call restart() yourself?
<re_irc>
<adamgreig> sounds nice
<re_irc>
<dirbaio> yup
<re_irc>
<adamgreig> I bet no one ever calls restart()
<re_irc>
<dirbaio> well, but it's there in case someone needs
<re_irc>
<dirbaio> +it
<re_irc>
<dirbaio> and it's not that much extra work
<re_irc>
<dirbaio> because HALs already have to support restarts if addr/dir changes
<re_irc>
<eldruin> is that a problem to implement for HALs, supporting some stray restart requests?
<re_irc>
<dirbaio> I guess the HALs will internally track some state about what was the "last" operation if any
<re_irc>
.restart() would just overwrite that, so the next operation always restarts
<re_irc>
<dirbaio> so operations know whether to restart or not
<re_irc>
<adamgreig> yea, seems like it would be easy to support once you have the rest
<re_irc>
<eldruin> hmm, one thing that you can do with transactional that you cannot do here is to iterate through operations within the same transaction (transaction_iter()) without async
<re_irc>
<dirbaio> (but there's no way to impl the current Transactional either, so... 🤷)
<re_irc>
<eldruin> dirbaio: what do you mean?
<re_irc>
<dirbaio> I mean this
<re_irc>
<eldruin> ah ok
<re_irc>
<dirbaio> nrf52 i2c doesn't allow you to send 2 separate buffers without a restart
<re_irc>
<dirbaio> which is stupid
<re_irc>
<eldruin> sorry I threw in a new topic
<re_irc>
<dirbaio> both the current Transactional API, and this new API allow that... so HALs will have to do hacks
<re_irc>
<dirbaio> maybe copying stuff to a temp buffer
<re_irc>
<dirbaio> eldruin: what do you mean?
<re_irc>
<dirbaio> inside the transaction() closure you can execute arbitrary code
<re_irc>
<dirbaio> you can iterate whatever
<re_irc>
<dirbaio> +you want
<re_irc>
<eldruin> yes, but you cannot provide new operations from the outside, because you are in a closure
<re_irc>
<dirbaio> you can pass an iterator from the outside, and iterate it from within the closure
<re_irc>
<eldruin> you can with async if you await foreign stuff, then continue sending it
<re_irc>
<dirbaio> you can impl the current Transactional API on top of this new API
<re_irc>
<eldruin> dirbaio: that is true
<re_irc>
<dirbaio> the new API is strictly more powerful
<re_irc>
<eldruin> yeah sounds good
<re_irc>
<dirbaio> oh boi
<re_irc>
<eldruin> sounds like we can make the API much simpler
<re_irc>
<eldruin> no more iter methods as well
<re_irc>
<dirbaio> I like it a lot
<re_irc>
<dirbaio> I'm excited :D :D
<re_irc>
<dirbaio> shall I PR it?
<re_irc>
<eldruin> indeed, me too
<re_irc>
<eldruin> yes!
<re_irc>
<adamgreig> nice :D
<re_irc>
<adamgreig> ok, we're way over time for the meeting 😅 thanks everyone!
<re_irc>
<dirbaio> it even allows removing read_write
<re_irc>
<dirbaio> just 2 methods!! :D
<re_irc>
<eldruin> yeah exactly
<re_irc>
<dirbaio> it even allows removing write_read
<re_irc>
<eldruin> 🤯
<re_irc>
<dirbaio> hm
<re_irc>
<dirbaio> I guess I2cBus will also need start(), stop()
<re_irc>
<dirbaio> or maybe just stop(), if read/write do the start if needed
<re_irc>
<adamgreig> hmm
<re_irc>
<adamgreig> you'd stop automatically at end of closure right?
<re_irc>
<eldruin> hmm, why would it?
<re_irc>
<adamgreig> or you mean I2CBus has stop() but not the device?
<re_irc>
<adamgreig> I guess if you had a stop() method akin to the restart() method, you could use it to do a start, addr, write, stop, start, addr, write, stop all in one "transaction"
<re_irc>
<dirbaio> this is if you use I2cBus directly, not through Device
<re_irc>
<dirbaio> * I2cDevice
<re_irc>
<dirbaio> remember, HALs impl only the Bus traits
<re_irc>
<dirbaio> Device impls are HAL-independent
<re_irc>
<adamgreig> 👍️
<re_irc>
<dirbaio> or that's the way I thought it for SPI, but I think the same works for I2c
<re_irc>
<eldruin> ok I got it now, but that's how the protocol works, if you are going the manual route I would say it is ok
<re_irc>
<dirbaio> so the Device impl would automatically call .stop() for you
<re_irc>
<dirbaio> but if you use the bare Bus then you have to do it manually
<re_irc>
<eldruin> yeah sounds good
<re_irc>
<dirbaio> 🚀
<re_irc>
<dirbaio> this new API won't work with today's shared-bus (lock per method call) though
<re_irc>
<dirbaio> shared-bus should switch to impl-ing Device
<re_irc>
<dirbaio> if you want sharing, use Device
<re_irc>
<dirbaio> * SpiDevice, I2cDevice
<re_irc>
<eldruin> I think this might be ok. at the end of the day, there is really only one bus. you can do all kinds of crazy stuff with it from inside the HAL anyway. I think this mechanism would reduce the need for the secretly sharing the bus for >95% of the cases
<re_irc>
<eldruin> that would be handled by the SPI device impl right? since it is in the HAL, and the HAL knows when it is still sending stuff and when it is done
<re_irc>
<eldruin> so the whole thing can work as-if
<re_irc>
<eldruin> even though some operations are deferred
<re_irc>
<dirbaio> SPI device impls can be hal-independent, the HALs only have to impl the bus
<re_irc>
<dirbaio> so bus needs a .flush() or something
<re_irc>
<eldruin> hmmm
<re_irc>
<dirbaio> or mandating read/write must block until fully done
<re_irc>
<eldruin> hmm difficult topic
<re_irc>
<dirbaio> uart already has a flush
<re_irc>
<dirbaio> and I guess the same problem applies to the new i2c api? allowing buffering is nice because it avoids gaps if you .write() multiple independent buffers
<re_irc>
<eldruin> hmm, for SPI, you would always need to call flush before deaserting CS
<re_irc>
<eldruin> and forgetting to call it would be a real footgun
<re_irc>
<dirbaio> Device impls would do it for you
<re_irc>
<dirbaio> the ones with ManagedCS at least
<re_irc>
<dirbaio> .flush() is like a "barrier"
<re_irc>
<dirbaio> "ensure all previous operations have fully completed"
<re_irc>
<firefrommoonlight> that sounds like a recipe for race conditions
<re_irc>
<dirbaio> and boom, you just bought yourself a first class ticket to garbage-city
<re_irc>
<firefrommoonlight> Shouldn't you be locking the Spi periph or something?
<re_irc>
<dirbaio> so
<re_irc>
<firefrommoonlight> (Or using a sep SPI periph in each thread lol!)
<re_irc>
<dirbaio> firefrommoonlight: ypi cam
<re_irc>
<dirbaio> * you can't if you want to share sck, miso, mosi to save on pins
<re_irc>
<dirbaio> which is a super common thing to do
<re_irc>
<firefrommoonlight> Yea this is use-case dependent
<re_irc>
<dirbaio> firefrommoonlight: that's exactly what SpiDevice does
<re_irc>
<dirbaio> the problem is "locking" itself is already a super complicated concept :'(
<re_irc>
<dirbaio> which has many possible implementations ☝️
<re_irc>
<firefrommoonlight> If you have a periph struct for Spi as an RTIC resource, your race condition should be blocked
<re_irc>
<firefrommoonlight> And if you want concurrent access eg via DMA or something, that's when I'd try to use 2 SPI periphs
<re_irc>
<firefrommoonlight> (If your pin count concern isn't an issue)
<re_irc>
<firefrommoonlight> I guess I tend not to look for generic solutions because they're so tough to get right!
<re_irc>
<dirbaio> firefrommoonlight: it's not. If you're doing everything interrupt-driven, an RTIC task can start a transfer and then return, then another task tries to use the same SPI
<re_irc>
you can avoid that by blocking, but then what's the point of using RTIC? :P
<re_irc>
<dirbaio> same with async
<re_irc>
<firefrommoonlight> Good point
<re_irc>
<firefrommoonlight> I assumed a naive blocking case
<re_irc>
<dirbaio> the thin gis
<re_irc>
<firefrommoonlight> You're right
<re_irc>
<dirbaio> * thing is
<re_irc>
person B writes a driver for Bar using the embedded-hal traits
<re_irc>
<dirbaio> person A writes a driver for Foo using the embedded-hal traits
<re_irc>
person C makes a PCB with a Foo chip and a Bar chip in the same SPI bus, then wants to use these drivers
<re_irc>
<firefrommoonlight> I think the _hard problem_ here is trying to use generic drivers
<re_irc>
<firefrommoonlight> Why they're nice is obvious - saves work if someone's already written one
<re_irc>
<firefrommoonlight> In practice, it seems like it's usually easier to skim the datasheet reg tables and do the writes and reads manually
<re_irc>
<dirbaio> depends!
<re_irc>
<dirbaio> if it's a super complex chip you'll have a bad time
<re_irc>
<firefrommoonlight> I'd guess a more complicated periph is when you'd want a generic driver
<re_irc>
<firefrommoonlight> *yea!
<re_irc>
<firefrommoonlight> I got this reply from an ST engineer about why a TOF sensor uses a C-lib API, and has no reg info in the datasheet:
<re_irc>
> The problem with the VL53L1s are their complexity. It's not one or two registers you have to twiddle, but thousands. When the chip was invented we put in so many registers so we could adjust the sensor because we didn't really know the answer. And we used c-code to converge on the right settings. Publishing the register map of hundreds and hundreds of registers would just cause you to run away. I suppose you could do it, but in...
<re_irc>
... then end you would hate ST.
<re_irc>
<dirbaio> lol :D
<re_irc>
<dirbaio> them not publishing a Rust driver also makes me hate ST 👍️
<re_irc>
<dirbaio> anyway
<re_irc>
<firefrommoonlight> No!
<re_irc>
<dirbaio> the whole point of embedded-hal is to allow writing these generic hal-independent drivers
<re_irc>
<firefrommoonlight> Later in the thread, the guy (Who wrote the datasheet and probably much of the chip design) tried to use a C to Rust automatic translater on it!
<re_irc>
<firefrommoonlight> But ran into problems with teh error types or something
<re_irc>
<dirbaio> wow, that's so cursed
<re_irc>
<firefrommoonlight> (I ended up hand translating the "lite" version of the driver; was a bit of work, but not too bad)
<re_irc>
<firefrommoonlight> Could have done FFI too, although not sure how it would work with the actual I2C writes (Which the C lib provides empty fns for you to fill in)
<re_irc>
<dirbaio> oof yeah
<re_irc>
<dirbaio> I did the "fill empty fns" for their st25r3916 C driver
<re_irc>
<dirbaio> heaps of unsafe
<re_irc>
<dirbaio> then it would interact badly with async because their C code wanted a pin interrupt and _would spin in main thread_ waiting for it
<re_irc>
<dirbaio> so it would hang if you processed the irq "later", it really had to preempt the main thread
<re_irc>
<firefrommoonlight> You can avoid teh unsafe if you replace pointer args with "&mut" or array refs etc
<re_irc>
<dirbaio> then I scrapped the whole thing and wrote my driver in rust lol
<re_irc>
<firefrommoonlight> Ooh - not sure how you'd handle interrupts using FFI or automatic translation
<re_irc>
<firefrommoonlight> I think you'd need to hand translate that to embassy, RTIC, cortex-m lib etc
<re_irc>
<dirbaio> their driver could "spin waiting for pin high"
<re_irc>
<dirbaio> but instead it did "spin waiting for a static flag that the pin irq sets"
<re_irc>
<dirbaio> like.. WHY
<re_irc>
<firefrommoonlight> ick
<cr1901>
>but instead it did "spin waiting for a static flag that the pin irq sets"
<cr1901>
I do that for a timer in some code I wrote, but I think it's an oversight I haven't fixed yet
<re_irc>
<dirbaio> we've all done ugly stuff
<re_irc>
<dirbaio> but if you do that in a driver specifically designed to be portable, you suck :D
<re_irc>
<dirbaio> * doing that in a driver specifically designed to be portable is crimilar
<cr1901>
You have to call timer_int in the timer interrupt handler, but at least it's isolated to a single HAL impl
<cr1901>
(and yes, this is contrived, but I'm sure I could come up with a useful interrupt handler)
<cr1901>
Also, doesn't some of the STM32 UART HAL impls allow you to use interrupts?
<re_irc>
<therealprof> cr1901: Which one doesn`t? You can always set up interrupts, use an interrupt handler and then use the primitive blocking impl in the interrupt handler to retrieve the data.