dc740 has quit [Remote host closed the connection]
starblue has quit [Ping timeout: 255 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 255 seconds]
starblue has joined #rust-embedded
fabic has joined #rust-embedded
starblue has quit [Ping timeout: 268 seconds]
starblue has joined #rust-embedded
<re_irc>
<@jamesmunns:beeper.com> Just a note , it's not _generally_ "best practice" to DMA structs specifically, e.g. memcpy the struct over the wire. If your type isn't "#[repr(C)]", it definitely is problematic (struct ordering/layout of non-repr-C types is undefined). It's usually not a bad idea to "serialize" types instead, copying the fields to a byte slice of some kind.
<re_irc>
<@jamesmunns:beeper.com> that being said, if you use repr(c) it's fine, just not super portable (endianness and length of fields like "usize" will cause problems, like in C). If you don't "observe" the padding bytes (if there are any), like memcpying the whole struct to a byte array, and looking at the padding bytes, you're probably fine (you're fine if the DMA is doing the copy for you and you never "see" the padding bytes)
starblue has quit [Ping timeout: 255 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 268 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 255 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 276 seconds]
starblue has joined #rust-embedded
cr1901_ has joined #rust-embedded
cr1901 has quit [Killed (NickServ (GHOST command used by cr1901_!~cr1901@2601:8d:8600:911:40ab:efe6:2026:d4c2))]
cr1901_ is now known as cr1901
starblue has quit [Ping timeout: 246 seconds]
starblue has joined #rust-embedded
fabic has quit [Ping timeout: 264 seconds]
starblue has quit [Ping timeout: 276 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 246 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 276 seconds]
starblue has joined #rust-embedded
starblue has quit [Ping timeout: 250 seconds]
starblue has joined #rust-embedded
cr1901 has quit [Read error: Connection reset by peer]
cr1901_ has joined #rust-embedded
fabic has joined #rust-embedded
starblue has quit [Ping timeout: 264 seconds]
starblue has joined #rust-embedded
fabic has quit [Ping timeout: 248 seconds]
fabic has joined #rust-embedded
starblue has quit [Ping timeout: 276 seconds]
rardiol has joined #rust-embedded
starblue has joined #rust-embedded
fooker has quit [Quit: WeeChat 3.7.1]
fooker has joined #rust-embedded
starblue has quit [Ping timeout: 255 seconds]
starblue has joined #rust-embedded
<re_irc>
<@firefrommoonlight:matrix.org> +1 on explicit serialization
IlPalazzo-ojiisa has joined #rust-embedded
fabic has left #rust-embedded [Leaving]
<re_irc>
<@dlaw:matrix.org> Thanks for the pointer. Performance considerations prohibit serializing a copy of the data in this case -- my struct fills > 50% of RAM and it's being blasted out an SPI at the core clock speed. I was sure to use repr(c).
GenTooMan has quit [Ping timeout: 260 seconds]
<re_irc>
<@ryan-summers:matrix.org> You could probably still implement a serializer that serialized directly into SPI output
<re_irc>
<@ryan-summers:matrix.org> That's an interesting idea :P
GenTooMan has joined #rust-embedded
IlPalazzo-ojiisa has quit [Read error: Connection reset by peer]
<re_irc>
<@adamgreig:matrix.org> hi @room, meeting time again! agenda is https://hackmd.io/lgsYC3imRnaF4-_38orogQ, please add anything you'd like to discuss and we'll start in a few minutes
<re_irc>
<@tippfehlr:matrix.org> Is there a library for arduino (Uno/Nano/328P) sleeping? (avr/sleep.h)
<re_irc>
<@dirbaio:matrix.org> modulo 's comment... but I don't think there's a good fix if we want the link to work from "docs.rs" or "crates.io", as relative links don't work there
<re_irc>
<@therealprof:matrix.org> : Probably takes some time getting used to. 😉
<re_irc>
<@dirbaio:matrix.org> we could have no link, but imo "link that points to latest master" is better than "no link"
<re_irc>
<@adamgreig:matrix.org> it's still just for nightly so not immediately relevant to using it on stable, whereas getting 63 released is more useful for that
rardiol has quit [Ping timeout: 265 seconds]
<re_irc>
<@adamgreig:matrix.org> if anyone has actually used or knows about the Allocator API I'd appreciate a review of #61 anyway, it looks straightforward enough but I don't know if there's anything in particular to watch out for
<re_irc>
<@adamgreig:matrix.org> ok, next topic, the question of svd2rust safe register access has reared its head again, I don't expect we'll find answers today but it's worth highlighting the issue thread https://github.com/rust-embedded/svd2rust/issues/714
<re_irc>
<@dirbaio:matrix.org> oh boi :D
<re_irc>
<@adamgreig:matrix.org> I think it's gotten a bit confused about where svd2rust makes things safe and the usual arguments about what's actually UB and a novel point about if reading should always be safe (it is currently always safe if you have the singleton, but reads can have side effects, but can reads ever have UB side effects? it seems unlikely)
<re_irc>
<@newam:matrix.org> I have seen UB from reads, but usually in big enterprise ASICs where there isn't as much verif.
<re_irc>
<@adamgreig:matrix.org> well, good
<re_irc>
<@adamgreig:matrix.org> I'm not sure about the idea of adding a flag to make all writes unsafe by default and nothing else, but I wonder if a flag to disable owned singletons (and thus make all access unsafe) would be an interesting direction
<re_irc>
<@dirbaio:matrix.org> i've already said it a few times
<re_irc>
<@dirbaio:matrix.org> imo the PAC is not the right layer to create safe APIs. the HAL is
<re_irc>
<@adamgreig:matrix.org> yea, but would you rather just see the next svd2rust verison entirely remove the owned singletons? my concern is for some amount of stability in such a widely used tool
<re_irc>
<@adamgreig:matrix.org> pretty much every HAL except embassy's will have to introduce their own way to handle owned singletons and stop expecting to be given them from the PAC, so it would be a fairly violent change, whereas a codegen flag might allow a softer transition?
rardiol has joined #rust-embedded
<re_irc>
<@dirbaio:matrix.org> personally for me, that ship has already sailed 😅
<re_irc>
<@adamgreig:matrix.org> I don't think I've seen many/any arguments in favour of the owned singletons beyond inertia at this point
<re_irc>
<@dirbaio:matrix.org> but yeah, having it as a flag makes sense, if the svd2rust maintainers want to maintain 2 different codegen styles
<re_irc>
<@dirbaio:matrix.org> what'll happen though is existing HALs will keep using the singletons
<re_irc>
<@dirbaio:matrix.org> so idk
<re_irc>
<@adamgreig:matrix.org> but I don't want the discussion around it to get confused about when access should be safe and what's UB and so on, I think the important distinction is just around what abstraction layer should handle ownership/safety
<re_irc>
<@adamgreig:matrix.org> we could eventually remove the flag and make that the only codegen if we wanted to force the issue, or if eg most PACs had migrated
<re_irc>
<@adamgreig:matrix.org> or maybe the important point is that the svd file alone doesn't tell a tool enough information to know whether access should be safe or not, and overloading writeConstraint and enumeratedValues for that purpose is not a complete answer
<re_irc>
<@dirbaio:matrix.org> there's also the drawback of ensuring safety when mixing HALs, or several versions of the same HAL
<re_irc>
<@dirbaio:matrix.org> the PAC singletons, for all their faults, do that fine
<re_irc>
<@dirbaio:matrix.org> there was that idea of creating "peripherals" crates, with just the singletons
<re_irc>
<@adamgreig:matrix.org> do people do that? besides mixing embassy and non-embassy HALs which seems to be the only recent cases I've spotted :P
<re_irc>
<@dirbaio:matrix.org> but i'm skeptical of that idea, because different HALs might want to split the singletons differently
<re_irc>
<@adamgreig:matrix.org> yea, that idea got bogged down in how to split interrupts and gpio pins and dma channels and so on
<re_irc>
<@adamgreig:matrix.org> binary blobs?
<re_irc>
<@dirbaio:matrix.org> : yeah, it's rare. Even on embassy, now that it's mature. I haven't seen see people asking about mixing HALs lately.
<re_irc>
<@therealprof:matrix.org> Yeah, custom communication firmwares running on the same core.
<re_irc>
<@adamgreig:matrix.org> as in, the svd2rust owned singletons break in that case, whereas a HAL might be able to know about them?
<re_irc>
<@therealprof:matrix.org> : No, it's a parallel universe (typically written in C) colliding with anything done on the Rust side. Not much difference between mixing Rust HALs and such abominations IMHO.
<re_irc>
<@adamgreig:matrix.org> I mean: svd2rust can never know about such a thing and will always just give you safe access as though you owned the peripherals, which is wrong, whereas a HAL for your chip could be told you're using that firmware, or detect it, or even provide it (eg nrf softdevice) and therefore know to coordinate/synchronise/prohibit access to those peripherals
<re_irc>
<@adamgreig:matrix.org> it seems like an argument in favour of svd2rust only doing unsafe access and HALs handling safety
<re_irc>
<@therealprof:matrix.org> : It is. What I was trying to say is: People do all kind of funky stuff, so whathever sense of safety we provide to them could be unjustified anyway.
<re_irc>
<@adamgreig:matrix.org> cool, well let's not rehash it again, it sounds like it would be worth putting together a concept for a codegen flag that removes the singletons and makes unsafe use simpler
<re_irc>
<@adamgreig:matrix.org> , do you wanna run through the embedded hal PRs?
<re_irc>
<@dirbaio:matrix.org> sure
<re_irc>
<@dirbaio:matrix.org> I'll try to go in increasing order of controversialness
<re_irc>
I think this one should be uncontroversial, it's just adding the async version of the already-existing Write.
<re_irc>
<@dkhayes117:matrix.org> Before the convo gets too deep, announcement: I've invited to join the RISC-V team. We have 2 out of 3 votes in favor, only waiting on @Disasm for his vote. 🎉
<re_irc>
<@adamgreig:matrix.org> technically 2/3 is a majority but might as well give a chance to see it :P
<re_irc>
<@dkhayes117:matrix.org> I forgot it didn't have to be unanimous
<re_irc>
<@adamgreig:matrix.org> it's been great to see a bunch of risc-v activity coming along!
<re_irc>
<@adamgreig:matrix.org> : does having separate write/flush make sense for async?
<re_irc>
<@adamgreig:matrix.org> I get that under the hood it might be that eg the last write has made it to the hardware fifo but not finished serialisation and maybe that's when the HAL wants to return
<re_irc>
<@adamgreig:matrix.org> hmm, and I guess if you want gapless transmission between multiple calls to write that's what you want to do...
<re_irc>
<@adamgreig:matrix.org> so yea, I guess even with async it's nicer to have it able to return early
<re_irc>
<@dirbaio:matrix.org> the async "Write" can return before the bytes are actualy tx'd yes
<re_irc>
<@dirbaio:matrix.org> just like the blocking one
<re_irc>
<@dirbaio:matrix.org> so if you want to actually wait until full TX, you call ".flush().await"
<re_irc>
<@adamgreig:matrix.org> it _can_, I was wondering if that ability still makes sense for async where it could easily keep being not ready until it's done
<re_irc>
<@adamgreig:matrix.org> like you could just as well specify in the trait that it must only return once transmission completes
<re_irc>
<@dirbaio:matrix.org> IMO blocking and async should work the exact same way
<re_irc>
<@adamgreig:matrix.org> but it seems like it is still useful to allow it to return before completion and have a separate flush, even on async, so
<re_irc>
<@dirbaio:matrix.org> either allow buffering, or not
<re_irc>
<@dirbaio:matrix.org> the current blocking trait does allow it, so I made the async trait allow it
<re_irc>
<@dirbaio:matrix.org> also this way it's consistent with SPI, which also allows buffering
<re_irc>
<@adamgreig:matrix.org> yea, and I think in both cases it's good to allow it, it matches common hardware well
<re_irc>
<@adamgreig:matrix.org> yea
<re_irc>
<@dirbaio:matrix.org> impls are also allowed to _not_ buffer, of course
<re_irc>
<@dirbaio:matrix.org> in which case "flush" is a noop
<re_irc>
<@dirbaio:matrix.org> 👉️ next up is I2C.
<re_irc>
After the discussions in the old i2c Bus/Device PR (https://github.com/rust-embedded/embedded-hal/pull/392), I don't think doing the split is a good idea anymore, the existing trait works fine for shared buses as-is.
<re_irc>
<@dirbaio:matrix.org> - implement all methods on top of "transaction"
<re_irc>
- remove "_iter" methods
<re_irc>
<@dirbaio:matrix.org> implement all methods on top of transaction -> this is a quality-of-life improvement for HALs. They only have to impl "transaction" and they get all the other methods for free. They can still choose to impl all methods, if they want to avoid the overhead of constructing the Operation slice for the other methods. I honestly can't think of any downsides. :)
<re_irc>
<@dirbaio:matrix.org> remove _iter methods -> this is consistent with what we've done with other traits.
<re_irc>
<@dirbaio:matrix.org> thoughts? :D
<re_irc>
<@therealprof:matrix.org> Sounds not too shabby. 😛
<re_irc>
<@dirbaio:matrix.org> the other PR is just adding docs, to document the fact that the trait is intended to be implemented for both exclusive and shared buses, and give some guidelines
<re_irc>
SpiDevice gets transaction, and the helper methods read/write/transfer/transfer_in_place that internally use transaction.
<re_irc>
<@adamgreig:matrix.org> it doesn't support someone needing to hold exclusive access to the bus while doing something else, but... hopefully that's not really a thing
<re_irc>
<@adamgreig:matrix.org> seems much less likely to be a thing with i2c than spi too
<re_irc>
<@adamgreig:matrix.org> : hmm....
<re_irc>
<@dirbaio:matrix.org> so linux spidev impls "SpiDevice"
<re_irc>
<@adamgreig:matrix.org> but SpiDevice would stop taking closures?
<re_irc>
<@adamgreig:matrix.org> yea
<re_irc>
<@dirbaio:matrix.org> and there's no support for the closure thing,y es
<re_irc>
<@dirbaio:matrix.org> * thing, yes
<re_irc>
<@adamgreig:matrix.org> that's maybe a touch annoying if you have something that needs SpiBus and then can't get it on Linux
<re_irc>
<@adamgreig:matrix.org> but I guess so it goes
<re_irc>
<@adamgreig:matrix.org> I think you could probably provide SpiBus on linux anyway, just with some more fiddling
<re_irc>
<@dirbaio:matrix.org> you could still get a SpiBus on linux I think?
<re_irc>
<@dirbaio:matrix.org> just the SPI thing with no CS
<re_irc>
<@dirbaio:matrix.org> * use the SPI kernel API,
<re_irc>
<@adamgreig:matrix.org> yea, but the kernel will also give someone else the same thing with a different cs, if you have two spidev on the same bus?
<re_irc>
<@adamgreig:matrix.org> so it might be you have to pinky promise there's no one else on the bus
<re_irc>
<@dirbaio:matrix.org> yeah, if you want a raw spi bus with no CS, you shouldn't be doing bus sharing
<re_irc>
<@dirbaio:matrix.org> SpiBus mandates there's no sharing
<re_irc>
<@adamgreig:matrix.org> right, but if you're on linux and want to use spi to drive a smart led
<re_irc>
<@adamgreig:matrix.org> and the smartled crate takes SpiBus because that's the right thing for it to do
<re_irc>
<@adamgreig:matrix.org> and you happen to know _your_ hardware only has one spidev for this bus, so it's perfectly fine to get a SpiBus from it
<re_irc>
<@dirbaio:matrix.org> yea
<re_irc>
<@dirbaio:matrix.org> "linux-embedded-hal" would provide a SpiDevice impl that is always safe to use
<re_irc>
<@adamgreig:matrix.org> but I don't know if linux-e-h could safely give out SpiBus
<re_irc>
<@adamgreig:matrix.org> yea
<re_irc>
<@dirbaio:matrix.org> and a SpiBus impl that's only safe to use if there's no sharing. Dunno if it can enforce that there's no sharing. if it can't, it could have some warning in the docs, perhaps make "new" unsafe.
<re_irc>
<@adamgreig:matrix.org> if spidevice did stop taking closures, do people lose any useful ability to do things while holding cs asserted? even if it might not work on linux
<re_irc>
<@dirbaio:matrix.org> I dunno. I've written a few drivers and haven't needed it...
<re_irc>
<@adamgreig:matrix.org> hmm, at least in principle it might be possible for it to check how many devices are on the bus you request and panic if it's >1, but you might have two devices in the platform tree but know only one is actually used, so I'd still want an unsafe escape hatch in that case
<re_irc>
<@adamgreig:matrix.org> but maybe a safe-and-checks-and-panics api is also possible
<re_irc>
<@dirbaio:matrix.org> most complex one is "cyw43", the transaction API has been nice to avoid copying data around
<re_irc>
<@adamgreig:matrix.org> I haven't looked at linux i2cdev in any detail, does it allow you to specify different addresses for every operation?
<re_irc>
<@dirbaio:matrix.org> which I think is the last outstanding issue for e-h 1.0
<re_irc>
<@dirbaio:matrix.org> : I don't think so, no?
<re_irc>
<@eldruin:matrix.org> : I think so
<re_irc>
<@dirbaio:matrix.org> lol
<re_irc>
<@dirbaio:matrix.org> you do one ioctl, then do reads/writes
<re_irc>
<@adamgreig:matrix.org> it seems like you ask the kernel for a new device with a particular address on a particular bus, and then you operate with that device
<re_irc>
<@dirbaio:matrix.org> in particular "ioctl(file, I2C_RDWR,...)" doesn't let you change the addr for each transfer
<re_irc>
<@adamgreig:matrix.org> so my worry was whether that's a problem if the i2c trait takes an address for every operation, rather than a sort of "device that has an address" and "bus that produces devices" split
<re_irc>
<@adamgreig:matrix.org> but I suppose you can just create the device for each transaction?
<re_irc>
<@adamgreig:matrix.org> oh, you can change it at runtime too
<re_irc>
<@dirbaio:matrix.org> : you open /dev/i2c-xxx once, then you can do an ioctl to set the address before each transaction
<re_irc>
<@adamgreig:matrix.org> there's an ioctl to change the address on a device
<re_irc>
<@adamgreig:matrix.org> yea, got it
<re_irc>
<@dirbaio:matrix.org> but you can't change within one transaction, it seems
<re_irc>
<@dirbaio:matrix.org> which mirrors what the current trait can do
<re_irc>
<@dirbaio:matrix.org> and I don't think we want that, it seems quite cursed
<re_irc>
<@adamgreig:matrix.org> indeed
<re_irc>
<@adamgreig:matrix.org> we're over time for this meeting, maybe let the new i2c prs sit for the week and we can try and get a decision on them next week?
limpkin has quit [Remote host closed the connection]
<re_irc>
<@adamgreig:matrix.org> it's possible to have a decision on them earlier too of course :P
<re_irc>
<@dirbaio:matrix.org> or some "r+"s on github 🙈
limpkin has joined #rust-embedded
<re_irc>
<@adamgreig:matrix.org> well, let's wrap up for now anyway, thanks everyone!
<re_irc>
<@adamgreig:matrix.org> making spidevice only take "&mut [Operation]"... 🤔
<re_irc>
<@dirbaio:matrix.org> 🤔 indeed
<re_irc>
<@dirbaio:matrix.org> I'm trying to prototype it
<re_irc>
<@dirbaio:matrix.org> there's one issue, which is it doesn't work with write-only or read-only buses
<re_irc>
<@dirbaio:matrix.org> onless we add
<re_irc>
WriteSpiDevice, with write_transaction, WriteOperation
<re_irc>
ReadSpiDevice with read_transaction, ReadOperation
<re_irc>
and
<re_irc>
<@dirbaio:matrix.org> it does work but it's quite a few methods
<re_irc>
<@adamgreig:matrix.org> ah yea, hm
<re_irc>
<@adamgreig:matrix.org> maybe not the end of the world given you'd still just have normal "transaction()" and "Operation" for the Read+Write case
<re_irc>
<@adamgreig:matrix.org> but I guess it does mean HALs have to implement all three transaction methods
<re_irc>
<@dirbaio:matrix.org> well, normally HALs impl just SpiBus
<re_irc>
<@dirbaio:matrix.org> so these 3 impls would be in embedded-hal-bus
<re_irc>
<@dirbaio:matrix.org> HALs that impl SpiDevice would have to impl all 3, yes
<re_irc>
<@adamgreig:matrix.org> that's not so different from having to implement SpiBusRead and SpiBusWrite and SpiBus methods I guess
<re_irc>
<@adamgreig:matrix.org> hmm, though I don't really get that, I wonder if we can provide an impl of ReadSpiDevice for SpiDevice and WriteSpiDevice for SpiDevice?
<re_irc>
<@adamgreig:matrix.org> anyway I'd better run
<re_irc>
<@dirbaio:matrix.org> : yes... but it'd look like the old "Default" traits, which weren't quite ergonomic/discoverable
<re_irc>
<@dirbaio:matrix.org> yeah, it is quite something, with 3 traits
<re_irc>
<@dirbaio:matrix.org> seems to work out nicely though...?
crabbedhaloablut has quit []
crabbedhaloablut has joined #rust-embedded
crabbedhaloablut has quit [Client Quit]
<re_irc>
<@dirbaio:matrix.org> and the cursed borrow issues on async are gone :D
crabbedhaloablut has joined #rust-embedded
crabbedhaloablut has quit []
crabbedhaloablut has joined #rust-embedded
<re_irc>
<@jannic:matrix.org> : Sorry, I didn't have time for the meeting today. Just one small remark: 63 only updates an example. It's not a requirement for using embedded-alloc on stable rust. That should just work out of the box.
Socke has quit [Ping timeout: 255 seconds]
cr1901 has quit [Read error: Connection reset by peer]