<re_irc>
<@whitequark:matrix.org> dirbaio: good job with recent smoltcp improvements!
<re_irc>
<@dirbaio:matrix.org> 😊 thanks!
<re_irc>
<@dirbaio:matrix.org> if you want, join #smoltcp:matrix.orgbtw
PyroPeter has quit [Ping timeout: 265 seconds]
PyroPeter has joined #rust-embedded
SanchayanMaity has quit [Ping timeout: 258 seconds]
SanchayanMaity has joined #rust-embedded
fabic has quit [Ping timeout: 260 seconds]
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 258 seconds]
unmanbearpig has quit [Ping timeout: 260 seconds]
unmanbearpig has joined #rust-embedded
<re_irc>
<@korken89:matrix.org> Hi ryan-summers I'm about to play with your `smoltcp-nal` a bit, but I'm not able to get it to compile. It seems that `smoltcp` has been updated to require a `rand` impl. Do you have something in the works for this or should I look at adding one? :)
<re_irc>
<@ryan-summers:matrix.org> I haven't gotten around to using the latest smoltcp with the NAL yet, so we could definitely use a rand impl. Would replace the PRNG it's currently using for tcp port numbers
<re_irc>
<@korken89:matrix.org> Makes sense!
<re_irc>
<@korken89:matrix.org> I'll play around a bit, see if I can get it figured out :)
<re_irc>
<@dirbaio:matrix.org> (unless you're using std, in which case it should use OsRng automatically)
<re_irc>
<@korken89:matrix.org> Awesome, thanks
<re_irc>
<@korken89:matrix.org> I'll just use something like `WyRng` when on embedded
<re_irc>
<@korken89:matrix.org> dirbaio: Does the rng need to be cryptographically secure
<re_irc>
<@dirbaio:matrix.org> No
<re_irc>
<@dirbaio:matrix.org> it's just used for tcp initial sequence numbers, dhcp transaction ids
<re_irc>
<@korken89:matrix.org> Ahh
<re_irc>
<@korken89:matrix.org> Cool
<Lumpio->
But those are important security keys!
<re_irc>
<@dirbaio:matrix.org> it's a very weak protection against blind packet spoofing, if you care about security you should be using TLS or some other strong crypto
<re_irc>
<@dirbaio:matrix.org> the rng will be used for a few more things in the future: DNS transaction ids and source ports, TCP source ports when unspecified
<re_irc>
<@dirbaio:matrix.org> but none requiring cryptographically-secure rng
<re_irc>
<@dirbaio:matrix.org> I probably should document that it doesn't need to be CS
<re_irc>
<@korken89:matrix.org> Awesome but then something simple like `WyRng` will be good enough :)
<re_irc>
<@dirbaio:matrix.org> yeah
<re_irc>
<@korken89:matrix.org> How is it with state? I'm thinking if one could plug in the MCU's HW rng
<re_irc>
<@korken89:matrix.org> But the interface does not have `self`
<re_irc>
<@dirbaio:matrix.org> just don't make it `return 0` or you may run into strange bugs
<re_irc>
<@dirbaio:matrix.org> yeah it's global, so that it doesn't infect everything with a generic param
<re_irc>
<@korken89:matrix.org> It seems like the macro generates a static function?
<re_irc>
<@korken89:matrix.org> Ahh I see
<re_irc>
<@korken89:matrix.org> Tradeoff
<re_irc>
<@dirbaio:matrix.org> yeah :S
<re_irc>
<@korken89:matrix.org> That's fine
<re_irc>
<@korken89:matrix.org> Then I understand what I have to work with :D
<re_irc>
<@dirbaio:matrix.org> if you want state you'll have to store it in a static yourself
<re_irc>
<@korken89:matrix.org> Indeed
<re_irc>
<@dirbaio:matrix.org> if you're using the HWRNG maybe you can get away with stealing it each time
<re_irc>
<@dirbaio:matrix.org> be careful, some HWRNGs are slow
<re_irc>
<@dirbaio:matrix.org> so maybe it's not worth it
<re_irc>
<@korken89:matrix.org> Yeah
<re_irc>
<@korken89:matrix.org> I'd rather seed a `WyRnd` with the HW rng
<re_irc>
<@korken89:matrix.org> That one is crazy fast
<re_irc>
<@korken89:matrix.org> (and small size)
fabic has joined #rust-embedded
<re_irc>
<@korken89:matrix.org> ryan-summers: I did a super simple impl for now, but I'm not sure how to protect the global from unsafe access.
<re_irc>
<@ryan-summers:matrix.org> Hah, I see you guys are having the same discussion I just started on the PR
<re_irc>
<@korken89:matrix.org> Haha
<re_irc>
<@ryan-summers:matrix.org> and yeah, mutex's in a hardware independent manner would be great. I also recognize how much a PITA it is
<re_irc>
<@korken89:matrix.org> Ops sorry for diverging the discussions
<re_irc>
<@dirbaio:matrix.org> it has builtin impls for std, cortex-m, riscv, wasm
<re_irc>
<@dirbaio:matrix.org> and can be customized with a `custom_impl!()` macro
<re_irc>
<@dirbaio:matrix.org> (that's actually where the idea for the smoltcp global rng comes from...)
<re_irc>
<@korken89:matrix.org> Ah I guess that would work
<re_irc>
<@dirbaio:matrix.org> I should write some docs, hehe 🙈
<re_irc>
<@korken89:matrix.org> Haha
<re_irc>
<@korken89:matrix.org> Damn users coming with questions
<re_irc>
<@ryan-summers:matrix.org> See PR for another idea
<re_irc>
<@ryan-summers:matrix.org> Essentially ignore the static mut and just construct once
<re_irc>
<@ryan-summers:matrix.org> If smoltcp is the only user of the rand impl, this problem goes away, so we can just shift everything over to smoltcp
<re_irc>
<@ryan-summers:matrix.org> But smoltcp-nal isn't really designed for that
<re_irc>
<@dirbaio:matrix.org> there *can* be multiple interfaces running at different priority levels but that'd be a weird thing to do lol
<re_irc>
<@ryan-summers:matrix.org> Yeah, I recognize it's not an impossibility, but it's something we could write docs around as "unsupported usage" for now until someone needs it
<re_irc>
<@korken89:matrix.org> So I guess, just to have a seed in the `new` would be fine then
<re_irc>
<@korken89:matrix.org> And then release all access to the rand
<re_irc>
<@ryan-summers:matrix.org> Yeah, that'd be the simplest approach for now. Add some docs saying not to use multiple smoltcp interfaces and open an issue to add support for that
<re_irc>
<@ryan-summers:matrix.org> Or just go with the critical section approach now and avoid the technical debt
<re_irc>
<@korken89:matrix.org> Makes sense
<re_irc>
<@dirbaio:matrix.org> changing smoltcp to use a more "normal" generics setup is still on the table btw
<re_irc>
<@korken89:matrix.org> I'll update the PR
<re_irc>
<@dirbaio:matrix.org> if the global rng turns out to suck
<re_irc>
<@ryan-summers:matrix.org> I think the current setup is fine, but it precludes safely having multiple Interface's constructed
<re_irc>
<@dirbaio:matrix.org> sprinkling generics everyhwere will suck too, the question is what sucks *less* haha
<re_irc>
<@korken89:matrix.org> Yeah
<re_irc>
<@ryan-summers:matrix.org> Because a single rand impl would always potentially pre-empt itself
<re_irc>
<@ryan-summers:matrix.org> this has nothing to do with the NAL
<re_irc>
<@dirbaio:matrix.org> you can have multiple interfaces just fine, you just have to mutex
<re_irc>
<@ryan-summers:matrix.org> Well, yeah
<re_irc>
<@ryan-summers:matrix.org> But if the mutex fails, its a runtime error, which is lame
<re_irc>
<@ryan-summers:matrix.org> You can't rely on something like RTIC to protect you
<re_irc>
<@ryan-summers:matrix.org> Or prevent it via static analysis
<re_irc>
<@ryan-summers:matrix.org> So there's no way to defend against the access violation
<re_irc>
<@ryan-summers:matrix.org> Unless that mutex is exposed
<re_irc>
<@dirbaio:matrix.org> with a critical section it shouldn't fail. it impacts interrupt latency though
<re_irc>
<@dirbaio:matrix.org> an alternative would be something like this
<re_irc>
<@dirbaio:matrix.org> I wonder if there's some clever way to need the generic only in Interface, and not in all of TcpSocket, UdpSocket, DnsSocket, DhcpSocket, SocketSet, SocketRef, Socket, SocketContext...
<re_irc>
<@korken89:matrix.org> `&dyn Rand` ?
<re_irc>
<@dirbaio:matrix.org> dyn sucks
<re_irc>
<@korken89:matrix.org> But it works ;)
<re_irc>
<@dirbaio:matrix.org> yeah..
<re_irc>
<@dirbaio:matrix.org> it's very annoying though
<re_irc>
<@dirbaio:matrix.org> because the Interface can't own it, just borrow it (you'd need Box)
<re_irc>
<@ryan-summers:matrix.org> AFK for a few minutes
<re_irc>
<@korken89:matrix.org> Yeah, storing it will be an issue
<re_irc>
<@korken89:matrix.org> And one can't add like a complie time allocator (= byte array with correct size and alignment) to store it in
<re_irc>
<@korken89:matrix.org> I used to do that trick a lot in C++, not sure if viable in Rust
<re_irc>
<@korken89:matrix.org> `std::aligned_storage_t` and friends
<re_irc>
<@dirbaio:matrix.org> if you need to parametrize it with the size somehow
<re_irc>
<@dirbaio:matrix.org> you may as well just have a good old generic param + field
<re_irc>
<@korken89:matrix.org> `[u8; core::size_of::<T>()]` might now be allowed yet
<re_irc>
<@korken89:matrix.org> Hmm, getting nice interfaces that does not pollute with generics all over the place is quite difficult
<re_irc>
<@dirbaio:matrix.org> it might be possible to have the generic just in the Interface struct, and in the XxxSocket process/emit *methods*, but not the struct
<re_irc>
<@korken89:matrix.org> Hmm
<re_irc>
<@korken89:matrix.org> Is it maybe simpler to commit to some rng impl?
<re_irc>
<@korken89:matrix.org> Just sidestep the problem
<re_irc>
<@dirbaio:matrix.org> yeah...
<re_irc>
<@dirbaio:matrix.org> just pass in a seed in Interface creation, and done
<re_irc>
<@korken89:matrix.org> Not ideal though
<re_irc>
<@korken89:matrix.org> Yeah
<re_irc>
<@korken89:matrix.org> I mean if it does not need to be cryptographically secure
<re_irc>
<@dirbaio:matrix.org> some people in that discussion didn't like hardcoding a non-cryptographically-secure algorithm
<re_irc>
<@korken89:matrix.org> Ahh
<re_irc>
<@dirbaio:matrix.org> but yeah there's not much benefit for it being cryptographically-secure
<re_irc>
<@korken89:matrix.org> Yeah I see their point
<re_irc>
<@korken89:matrix.org> But yeah, what I did in the PR will work for now
<re_irc>
<@korken89:matrix.org> I don't have good input on making it as simple and yet to not pollute the API
<re_irc>
<@dirbaio:matrix.org> perhaps just hardcoding some sumb PRNG
<re_irc>
<@dirbaio:matrix.org> like, the tcp initial seqno was hardcoded to `42` before
<re_irc>
<@dirbaio:matrix.org> any dumb PRNG is better than that
<re_irc>
<@dirbaio:matrix.org> lol
<re_irc>
<@korken89:matrix.org> Haha
<re_irc>
<@korken89:matrix.org> Indeed, I mean anything would suffice IMO as long as you as the user can seed it
<re_irc>
<@dirbaio:matrix.org> yeah, if the device reboots and the seed is static, smoltcp will pick the exact same port+seqno as the previous boot which will confuse the peer to hell
<re_irc>
<@dirbaio:matrix.org> dirbaio:matrix.org: yep, this works. The problem is it'll prevent a possible future refactor to do `dyn Socket`
<re_irc>
<@dirbaio:matrix.org> u128, lol
<re_irc>
<@korken89:matrix.org> :D
<re_irc>
<@korken89:matrix.org> There might be OK u32 SW rands as well?
<re_irc>
<@korken89:matrix.org> I always fallback to WyRand
<re_irc>
<@ryan-summers:matrix.org> I think the discussion pointed out how a non-CSPRNG could be an attack vector for DNS-based vulnerabilities
<re_irc>
<@ryan-summers:matrix.org> Which was why it was advised to not implement hard-coded non-CSPRNG
<re_irc>
<@dirbaio:matrix.org> IMO it's a bit paranoid
<re_irc>
<@ryan-summers:matrix.org> I've learned to be a bit paranoid. It's hard to tell how users are going to be using a lib, so just someone using smoltcp for e.g. electronic locks, that could expose an attack vector to physically enter buildings via a DNS exploit
<re_irc>
<@ryan-summers:matrix.org> If they're grabbing a database e.g. from some external server
<re_irc>
<@dirbaio:matrix.org> 1. entropy is only 16bit source port + 16bit txid = 32bit, so an attack still has non-negligible probability of succeeding even with a CSPRNG
<re_irc>
<@dirbaio:matrix.org> 2. it only protects against blind spoofing. Someone that can see your traffic can just see the sport+txid and spoof a response 100% of the time
<re_irc>
<@dirbaio:matrix.org> tldr DNS is insecure anyway :D
<re_irc>
<@ryan-summers:matrix.org> I don't disagree with that assessment either
<re_irc>
<@dirbaio:matrix.org> so you should be using TLS anyway
<re_irc>
<@dirbaio:matrix.org> at which point DNS spoofing becomes just a DoS
<re_irc>
<@dirbaio:matrix.org> and someone that can spoof packets can DoS you easily in other ways
<re_irc>
<@ryan-summers:matrix.org> Is DNS-over-TLS widespread yet?
<re_irc>
<@ryan-summers:matrix.org> I remember there was a push for that from google/firefox last year or so
<re_irc>
<@ryan-summers:matrix.org> I think
<re_irc>
<@dirbaio:matrix.org> DoH kinda is, many resolvers like cloudflare's 1.1.1.1 support it
<re_irc>
<@dirbaio:matrix.org> DoT is less common
<re_irc>
<@dirbaio:matrix.org> for DoH you need to speak DNS, TLS, HTTP and JSON
<re_irc>
<@dirbaio:matrix.org> so, crazy bloated, and out of scope of core smoltcp
<re_irc>
<@dirbaio:matrix.org> also
<re_irc>
<@dngrs:matrix.org> dirbaio:matrix.org: does sccache help here?
<re_irc>
<@dirbaio:matrix.org> I gave it a quick try, it seemed broken (some "rustc not found" error)
<re_irc>
<@dngrs:matrix.org> aw crap, probably not because CI, ephemeral, bleh
<re_irc>
<@dirbaio:matrix.org> but I want to give it a serious try again later
<re_irc>
<@dirbaio:matrix.org> because it seems useful
<re_irc>
<@dngrs:matrix.org> clearly you need to *add more cloud*
<re_irc>
<@dirbaio:matrix.org> that post is pre-defmt though. IMO with defmt it's a solved problem
<re_irc>
<@dirbaio:matrix.org> the only papercut left is bloat from panics from .unwrap() or array indexing
<re_irc>
<@dirbaio:matrix.org> but that can be removed with `-Zbuild-std=core, -Zbuild-std-features=panic_immediate_abort`
<re_irc>
<@adamgreig:matrix.org> lachlansneff:matrix.org: that doesn't sound right... what pitch BGA? JLC will do a 4L process suitable for 0.8mm BGA for $2
<re_irc>
<@korken89:matrix.org> Oh, how does it solve the issue?
<re_irc>
<@korken89:matrix.org> dirbaio:matrix.org: This I mean
<re_irc>
<@dirbaio:matrix.org> even if you use panic-reset or similar "no-fmt" panic handler, the compiler can't see all the fmt stuff is unused and will still include some bloat
<re_irc>
<@dirbaio:matrix.org> `panic_immediate_abort` makes panics abort earlier so the compiler can now see it and optimize it all out
<re_irc>
<@korken89:matrix.org> Oh
<re_irc>
<@dirbaio:matrix.org> with it, panics compile to a single `udf` instruction :D
<re_irc>
<@dirbaio:matrix.org> 2 bytes
<re_irc>
<@korken89:matrix.org> Is that nightly only?
<re_irc>
<@dirbaio:matrix.org> (and that fw has most unwraps as `defmt::unwrap!()` already. win % will be higher in fws that don't)
<re_irc>
<@henrikssn:matrix.org> dirbaio:matrix.org: I didn't even know this existed
<re_irc>
<@adamgreig:matrix.org> hi room! meeting time again, agenda's https://hackmd.io/6l-V56rjQZSqs0JjjPmSqw, please add anything you'd like to discuss, we'll start in about 5min
<re_irc>
<@adamgreig:matrix.org> ...or as soon as I finish eating dinner :P
<re_irc>
<@adamgreig:matrix.org> nom nom
<re_irc>
<@adamgreig:matrix.org> ok, let's start! any announcements from anyone?
<re_irc>
<@adamgreig:matrix.org> reminder that rust 1.56 is out this thursday with edition 2021 then :o
<re_irc>
<@adamgreig:matrix.org> and also stabilising adding linker scripts from build scripts I believe...
<re_irc>
<@adamgreig:matrix.org> no more will .cargo/config.toml torment us
<re_irc>
<@dirbaio:matrix.org> and you can now have different linker scripts for different binaries and other cool stuff
<re_irc>
<@matoushybl:matrix.org> adamgreig: is there any more detail on it?
<re_irc>
<@hargonix:matrix.org> I'd imagine since the release is so close upstream its probably already implementd and documented?
<re_irc>
<@therealprof:matrix.org> ... and const_panic... 💃
<re_irc>
<@therealprof:matrix.org> Now we just need the ability to fast forward a few months so we can actually make use of all the gooddies MSRV wise. 😉
<re_irc>
<@adamgreig:matrix.org> hehe, should just about give us enough time to write some PRs :P
<re_irc>
<@hargonix:matrix.org> maybe some winter hibernation...
<re_irc>
<@adamgreig:matrix.org> ugh yes I'm already starting, not looking forward to clocks changing
<cr1901>
I hope eventually it becomes unnecessary to have to copy bits and pieces of a linker script around to form a full one, but Idk what that'd look like
<re_irc>
<@adamgreig:matrix.org> of which I think remaining Qs are "should it enforce read and write are same len, or let them differ?"
<re_irc>
<@adamgreig:matrix.org> and "can we try implementing it somewhere"
<cr1901>
ahh whoops
<re_irc>
<@dirbaio:matrix.org> I have on my todo list since a while ago doing the impls for embassy-nrf, embassy-stm32 of that
<re_irc>
<@dirbaio:matrix.org> so I guess we'll see
<re_irc>
<@dirbaio:matrix.org> no other volunteers it seems :)
<re_irc>
<@adamgreig:matrix.org> on the buffer length thing, it seems to me like it makes sense to allow them to differ, but implementations might have to pad the tx buffer with 0s to make that work, and then it's tricky to DMA...
<re_irc>
<@dirbaio:matrix.org> > have to pad the tx buffer with 0s to make that work
<re_irc>
<@adamgreig:matrix.org> but the convenience of having a short write of just a byte or two command, and a long read for the response, is nice
<re_irc>
<@dirbaio:matrix.org> cpu-copy impls can just write zeros
<re_irc>
<@adamgreig:matrix.org> yep true
<re_irc>
<@dirbaio:matrix.org> dma impls can split the transfer in a "bidirectional" part and a "tx-only" part
<re_irc>
<@adamgreig:matrix.org> DMA impls will probably still need a fake tx buffer though?
<re_irc>
<@eldruin:matrix.org> some dma imps could also just return an error if the sizes differ
<re_irc>
<@dirbaio:matrix.org> some dma impls already do that "by default": nrf5x, nrf91
<re_irc>
<@dirbaio:matrix.org> adamgreig: I think you usually can setup DMA to only tx or only rx. stm32 can
<re_irc>
<@adamgreig:matrix.org> I can also imagine buggy HALs that only send the send buffer then stop, so only first few bytes of receive are written to, at the least we should document it carefully
<re_irc>
<@adamgreig:matrix.org> hmm
<re_irc>
<@adamgreig:matrix.org> have you tried DMA rx-only SPI on stm32?
<re_irc>
<@dirbaio:matrix.org> eldruin:matrix.org: that'd be annoying, because then drivers can't rely on it working
<re_irc>
<@dirbaio:matrix.org> the trait should either mandate support or forbid it IMO
<re_irc>
<@adamgreig:matrix.org> I think the DRQ comes from the DR getting filled after a word is exchanged, but I wouldn't have expected the RX DMA to drive the transmission of a word
<re_irc>
<@adamgreig:matrix.org> (for one thing, how would it know what to send?)
<re_irc>
<@adamgreig:matrix.org> but, I haven't tried it
<re_irc>
<@dirbaio:matrix.org> I think there are some bits in the SPI peri you can tweak
<re_irc>
<@dirbaio:matrix.org> but maybe it woks for txonly but not rxonly, not sure
<re_irc>
<@adamgreig:matrix.org> I can definitely see txonly working fine
<re_irc>
<@dirbaio:matrix.org> if it's not possible to do dma rxonly, you can set the tx dma channel to a buffer with a single zero and disable memory adress increment
<re_irc>
<@adamgreig:matrix.org> ok, so conclusion is "do allow them to differ, document this, require implementations support any combination of lengths"?
<re_irc>
<@adamgreig:matrix.org> cool, ok
<re_irc>
<@adamgreig:matrix.org> would you mind summarising on the PR?
<re_irc>
<@dirbaio:matrix.org> LGTM to add Unknown if linux i2c is soooo broken
<re_irc>
<@adamgreig:matrix.org> given new evidence that there are indeed devices that _cannot_ distinguish data/addr nak, yea
<re_irc>
<@adamgreig:matrix.org> well question is do we add Unknown or do we just merge them all into a single Nak error
<re_irc>
<@adamgreig:matrix.org> if a driver wants to check if a device is present, it can always do a zero-data transaction and if that gets nak'd, it must be the address getting nak'd
<re_irc>
<@therealprof:matrix.org> I'd go single NAK.
<re_irc>
<@dirbaio:matrix.org> maybe docs wording could be a bit stronger? "if the underlying hardware/OS supports distinguishing, impls MUST return AddressNak/DataNak and not Unknown"
<re_irc>
<@adamgreig:matrix.org> and otherwise, the driver needs to handle both Unknown and each specific type
<re_irc>
<@dirbaio:matrix.org> to avoid the "lazy impls" problem
<re_irc>
<@adamgreig:matrix.org> it might help the lazy impls problem, though it's not like rustc can check
<re_irc>
<@adamgreig:matrix.org> but drivers presumably still have to handle the Unknown case somehow
<re_irc>
<@jannic:matrix.org> Is it guaranteed that there are no devices where zero-byte transactions have some side effect?
<re_irc>
<@adamgreig:matrix.org> (or just give up and refuse to work)
<re_irc>
<@adamgreig:matrix.org> haha, I'm sure there are some devices where zero-byte transactions have _some_ side effect
<re_irc>
<@adamgreig:matrix.org> hopefully not the same ones that might nak a data phase to ask you to try again later
<re_irc>
<@dirbaio:matrix.org> drivers that don't care which NAK can match on `NoAcknowledge(_)`
<re_irc>
<@adamgreig:matrix.org> but drivers that do care need to always handle unknown, and if they can handle it, why bother with separate types at all?
<re_irc>
<@dirbaio:matrix.org> drivers that do will have to handle Unknown. They can either handle it in "the most conservative way" or propagate it up as in "I can't work on this hardware"
<re_irc>
<@adamgreig:matrix.org> (unless those drivers just refuse to work with impls that return unknown, but that kinda sucks and also would have to be a runtime error?)
<re_irc>
<@therealprof:matrix.org> It's not ideal.
<re_irc>
<@adamgreig:matrix.org> at this point, do you think there's enough benefit to being able to signal different nak types?
<re_irc>
<@dirbaio:matrix.org> it's nice to have
<re_irc>
<@dirbaio:matrix.org> for human debugging alone it's useful
<re_irc>
<@dirbaio:matrix.org> and if a device that does require distinguishing in the future comes up, we'll be able to write a driver for it
<re_irc>
<@therealprof:matrix.org> I don't see how if it doesn't actually get implemented.
<re_irc>
<@dirbaio:matrix.org> if we don't add the variants now, we can't add them later (breaking change)
<re_irc>
<@adamgreig:matrix.org> true, it would be a nice help with debugging
<re_irc>
<@jannic:matrix.org> Are there established C APIs we could copy from?
<re_irc>
<@therealprof:matrix.org> Why would it a breaking change? That's the whole point of non_exhaustive...
<re_irc>
<@adamgreig:matrix.org> I think that's assuming we either just have a single Nak variant in the ErrorKind, vs the Nak enum proposed in the PR that itself has Addr, Data, and Unknown variants
<re_irc>
<@adamgreig:matrix.org> no point having the enum with a single variant
<re_irc>
<@adamgreig:matrix.org> we could add more Naks to the ErrorKind enum later, without breaking, but it's less ergonomic
<re_irc>
<@therealprof:matrix.org> There wasn't a lot of demand for years now, I'm not sure what would prompt that demand in the future. If someone desperately needed it, I'm sure they'd have complained.
<re_irc>
<@ryan-summers:matrix.org> As a developer, all I've ever cared about is the fact I got a NAK.
<re_irc>
<@dirbaio:matrix.org> if we launch ErrorKind with NoAcknowledge, drivers will handle that
<re_irc>
<@dirbaio:matrix.org> if we later add NoAcknowledgeAddress, NoAcknowledgeData then HALs will switch from NoAcknowledge to that, and not-updated drivers will now treat them as "unknown" errors, not NAK
<re_irc>
<@ryan-summers:matrix.org> If you need to differentiate e.g. EEPROM busy with write, you can do special polling loops just detecting the NACK and work around it
<re_irc>
<@dirbaio:matrix.org> it's not "it won't build" breaking, but it's "it won't work" breaking
<re_irc>
<@therealprof:matrix.org> Yeah, but it adds complexity for unclear gains. I'd be happy to see it added if someone stepped up with a real usecase which would be painful to implement without.
<re_irc>
<@dirbaio:matrix.org> non_exhaustive doesn't help in this case
<re_irc>
<@dirbaio:matrix.org> just for human debugging it's very useful!
<re_irc>
<@dirbaio:matrix.org> writing a driver
<re_irc>
<@adamgreig:matrix.org> we already argued about the use cases for distinguishing when the two types were put in the current merged enum
<re_irc>
<@adamgreig:matrix.org> the PR's about moving those to a separate enum to support an unknown variant for the devices which it turns out cannot differentiate
<re_irc>
<@adamgreig:matrix.org> I think the question is whether it's still worth distinguishing them in face of some devices not being able to, because they'll have to report this 'unknown' type
<re_irc>
<@dirbaio:matrix.org> got AddressNack? you got the wiring or address wrong
<re_irc>
<@dirbaio:matrix.org> got DataNack? you got the address right, you're probably sending a bad command byte or too much data
<re_irc>
<@dirbaio:matrix.org> that's super nice to know
<re_irc>
<@adamgreig:matrix.org> but it _is_ nice that with the new enum, a driver can match on the generic no-acknowledge error more ergonomically than naming both types from currently
<re_irc>
<@dirbaio:matrix.org> vs "you did *something* wrong, fuck you"
<re_irc>
<@adamgreig:matrix.org> yea, I am compelled by the ease of debugging...
<re_irc>
<@therealprof:matrix.org> language...
<re_irc>
<@dirbaio:matrix.org> it has certainly helped me in the past
<re_irc>
<@therealprof:matrix.org> adamgreig: Only if implemented.
<re_irc>
<@adamgreig:matrix.org> but why wouldn't it be?
<re_irc>
<@adamgreig:matrix.org> for _most_ implementations, I mean
<re_irc>
<@adamgreig:matrix.org> I agree in some cases it can't be, unfortunately
<re_irc>
<@therealprof:matrix.org> Because the HAL can't support it.
<re_irc>
<@adamgreig:matrix.org> but at least all the popular cortex-m microcontrollers seem to
<re_irc>
<@adamgreig:matrix.org> I expect in general most i2c peripherals would be able to, but seemingly linux will struggle
<re_irc>
<@dirbaio:matrix.org> Linux is the only thing we've found that doesn't support it.
<re_irc>
<@eldruin:matrix.org> for ease of debugging it would indeed be useful
<re_irc>
<@therealprof:matrix.org> I haven't missed it but hey...
<re_irc>
<@adamgreig:matrix.org> it hasn't had a chance to be missed yet :P
<re_irc>
<@therealprof:matrix.org> Just saying, you know what kind of NACK it is simply due to when it happens.
<re_irc>
<@adamgreig:matrix.org> are you more concerned about the extra complexity in the HAL impls or do you not think the end users will benefit?
<re_irc>
<@adamgreig:matrix.org> the HAL does, but the users of the HAL don't, right?
<cr1901>
I'm gonna be honest: I've read the scrollback up to this point, and I'm still not sure what the problem is compared to having a single enum vs splitting into variants
<re_irc>
<@adamgreig:matrix.org> they say "please send this data to this address", HAL says "nak", you can't tell if it was an addr or data nak (but the HAL probably could have told)
<re_irc>
<@therealprof:matrix.org> Usually you initialise devices and when the first send failes you know it's because the address isn't on the bus.
<re_irc>
<@adamgreig:matrix.org> but it could be because you sent a bad command or the device was busy, right?
<re_irc>
<@dirbaio:matrix.org> irc_libera_cr1901:psion.agg.io: discussion is whether to have variants for Address/Data NAKs, or just one generic NAK variant (in which case drivers can't know which kind it is at all)
<re_irc>
<@jannic:matrix.org> For linux, `Documentation/i2c/fault-codes.rst` says that a NAK on the address should return NXIO. So even linux seems to have separate error codes.
<re_irc>
<@therealprof:matrix.org> adamgreig: After starting? If that happens then you better hope the driver can handle that, otherwise you're toast anyway...
<cr1901>
Why not "NoAcknowledgeData, NoAcknowledgeAddress, NoAcknowledgeUnknown"?
<re_irc>
<@adamgreig:matrix.org> jannic: huh, we tried to find something about that earlier and couldn't find anything explicit, but it seems the many different linux i2c drivers will often do different things
<re_irc>
<@therealprof:matrix.org> irc_libera_cr1901:psion.agg.io: Because then everyone needs to handle three variants (if they care).
<re_irc>
<@adamgreig:matrix.org> though yea, nice to compare that the linux api does also distinguish them
<re_irc>
<@dirbaio:matrix.org> jannic:matrix.org: no mention what it returns on NAK on a data byte. Some drivers return EIO, some return ENXIO. Some return EIO on address NAK too, contradicting the docs.
<re_irc>
<@adamgreig:matrix.org> cr1901: with the second enum, it's easy to match on just `NoAcknowledge(_)` if you don't care, instead of needing to pattern match all three variants
<cr1901>
Ahhh
<re_irc>
<@jannic:matrix.org> Well the one driver I looked at used ENXIO for NAK on address and EIO for NAK on data. But well, inconsistent implementations are quite probably for linux drivers.
<re_irc>
<@dirbaio:matrix.org> also EIO is "generic io error", it can be something other than Data NAK
<cr1901>
Well, it doesn't bother me to handle 3 variants personally
<re_irc>
<@dirbaio:matrix.org> so you can't even reliably tell "was this a NAK, or something else?" 🤣
<re_irc>
<@dirbaio:matrix.org> like, if it's a USB-to-I2C thingy, unrelated USB errors will also cause EIO
<re_irc>
<@dirbaio:matrix.org> tldr: it's a mess
<re_irc>
<@adamgreig:matrix.org> thanks linux
<re_irc>
<@adamgreig:matrix.org> ok, let's revisit the nak thing later I think, I wanted to chat about a couple cortex-m things briefly in the time we have left
<re_irc>
<@jannic:matrix.org> Perhaps there should be some kind of `give_back()` to revert a `steal()`?
<re_irc>
<@adamgreig:matrix.org> > One issue that came up was that Peripherals::steal() is not documented to have any relationship with Peripherals::take(). In reality, calling steal() will cause take() to return None, and this is relied on by RTFM for soundness!
<re_irc>
<@dirbaio:matrix.org> also making steal() side-effect-free allows adding more convenience stuff like `SCB::steal()`, `NVIC::steal()`
<re_irc>
<@therealprof:matrix.org> I'm not saying I agree with the concept. I'm merely telling you what the idea was.
<re_irc>
<@therealprof:matrix.org> Pretty sure that explanation is buried somewhere in the WG repos.
<re_irc>
<@adamgreig:matrix.org> so, the current behaviour is relied on by crates like RTIC
<cr1901>
>Peripherals::steal() is not documented to have any relationship with Peripherals::take().
<cr1901>
This seems non-ideal; I only knew this because of reading the source code of generated PACs
<re_irc>
<@dirbaio:matrix.org> is it still true that RTIC relies on it?
<re_irc>
<@adamgreig:matrix.org> who want some way to say "mark the peripherals as taken" but avoid the CS of actually `take()`ing (maybe there's some other reason to avoid take?)
<re_irc>
<@dirbaio:matrix.org> and *why* would RTIC rely on it? lol
<re_irc>
<@therealprof:matrix.org> Because the original author designed it that way? 😉
<re_irc>
<@dirbaio:matrix.org> RTIC can call take().unwrap(). User took the peripherals? panic. User stole the peripherals? we have duplicates, but the responsibility for that is on the user
<re_irc>
<@adamgreig:matrix.org> yea, but now RTIC has an unavoidable `unwrap()` which I think they want to avoid
<re_irc>
<@adamgreig:matrix.org> user can't have taken the peripherals yet, because rtic happens before user code
<re_irc>
<@adamgreig:matrix.org> but anyway yea, basically cortex-m's api here is poorly documented and/or surprising to boot
<re_irc>
<@adamgreig:matrix.org> it's already an open issue to do _something_ about it, such as a proposed `conjure()` method (though i didn't like the name personally) that just makes a new Peripherals
<re_irc>
<@adamgreig:matrix.org> afaik there's no one besides rtic relying on it, but rtic definitely does rely on it for safety
<re_irc>
<@dirbaio:matrix.org> :(
<re_irc>
<@adamgreig:matrix.org> so, yea...
<re_irc>
<@dirbaio:matrix.org> so
<re_irc>
<@adamgreig:matrix.org> short-term, I think an unsafe fn in cortex-m that creates the peripherals without setting taken might be an easy addition for a point release
<re_irc>
<@dirbaio:matrix.org> invent a new name, set the convention that "it creates an instance unsafely without side effects", add it everwhere, deprecate steal()?
<re_irc>
<@adamgreig:matrix.org> changing what steal does is probably bad
<re_irc>
<@adamgreig:matrix.org> don't think it needs to be deprecated necessarily? but does need to be documented
<re_irc>
<@dirbaio:matrix.org> it's very confusing
<re_irc>
<@dirbaio:matrix.org> if you want to set the flag, you can just do `take()`
<re_irc>
<@adamgreig:matrix.org> lol
<re_irc>
<@adamgreig:matrix.org> only because `take()` literally just calls `steal()`
<re_irc>
<@adamgreig:matrix.org> `steal()` is the only thing that sets the flag atm
<re_irc>
<@dirbaio:matrix.org> lol
<re_irc>
<@dirbaio:matrix.org> but PACs also do it...
<re_irc>
<@dirbaio:matrix.org> so it is more widespread than I thought
<re_irc>
<@dirbaio:matrix.org> and PACs take forever to update
<re_irc>
<@adamgreig:matrix.org> PACs do it because both were written by the same people at the same time, and cortex-m should really be a PAC (at least this part of it)
<re_irc>
<@dirbaio:matrix.org> so maybe it's best to leave it as it is, otherwise it'll create more confusion if some `steal`s set the flag and others don't lol
<re_irc>
<@adamgreig:matrix.org> well that's why I'd suggest some different name that doesn't set the flag, and leave steal as-is
<re_irc>
<@dirbaio:matrix.org> and embassy is inconsistent, I guess I should find another name yeah
hifi has quit [Remote host closed the connection]
hifi has joined #rust-embedded
<re_irc>
<@adamgreig:matrix.org> (with better documentation than "Unchecked version of `Peripherals::take`" lol
<re_irc>
<@jannic:matrix.org> Could the alternative function, which doesn't set the flag, just be called `copy()`?
<re_irc>
<@jannic:matrix.org> (or clone)
<cr1901>
><@dirbaio:matrix.org> but PACs also do it...
<cr1901>
Well, isn't that because "steal/take" is autogenerated from svd2rust?
<re_irc>
<@dirbaio:matrix.org> "copy something from thin air" is strange
<re_irc>
<@dirbaio:matrix.org> I thought PACs's steal didn't set the flag when I made that cortex-m issue, I thought it was just cortex-m
<re_irc>
<@adamgreig:matrix.org> irc_libera_cr1901:psion.agg.io: unfortunately cortex-m isn't generated by svd2rust, despite also having a Peripherals struct (hand-written), and steal/take, etc
<re_irc>
<@dirbaio:matrix.org> rob(), conjure()..
<re_irc>
<@dirbaio:matrix.org> unsafe_new()
<cr1901>
adamgreig: Oh... :(
<re_irc>
<@adamgreig:matrix.org> the conclusion from the last time we had this naming debate was most of the names suck lol
<re_irc>
<@dirbaio:matrix.org> yeah the `steal` name is so goooood... shame
<cr1901>
steal() for not setting take, steal_as_well() for setting take. Who cares about backcompat :P?
<re_irc>
<@adamgreig:matrix.org> `steal2()` D:
<re_irc>
<@adamgreig:matrix.org> ok I've seen the word steal too much and it looks like steel spelt wrong, we're out of meeting time lol
<re_irc>
<@dirbaio:matrix.org> most people steal PAC peripherals with `&*pac::FOO::ptr()`
<re_irc>
<@jannic:matrix.org> `forge()`
<re_irc>
<@adamgreig:matrix.org> yea and it sucks huh
<re_irc>
<@adamgreig:matrix.org> plus you only get a RegisterBlock
<re_irc>
<@dirbaio:matrix.org> ah lol right it's not the same thing
<re_irc>
<@dirbaio:matrix.org> then `mem::transmute(())`, which looks even more arcane black magic
<re_irc>
<@dirbaio:matrix.org> having per-peripheral steal would be handy (both in PACs and cortex-m)
<re_irc>
<@adamgreig:matrix.org> yea, agreed
<re_irc>
<@adamgreig:matrix.org> might even remove the need for a new Peripherals steal
<re_irc>
<@adamgreig:matrix.org> though possibly too confusing to have `steal()` on each peripheral that doesn't set any flags and `steal()` on the `Peripherals` that does
<re_irc>
<@dirbaio:matrix.org> yeah..
<re_irc>
<@dirbaio:matrix.org> 🤷
<re_irc>
<@dirbaio:matrix.org> btw
<re_irc>
<@dirbaio:matrix.org> thoughts on unifying the SPI traits?
<re_irc>
<@dirbaio:matrix.org> on just 3 traits: readonly, writeonly, bidirectional
<re_irc>
<@adamgreig:matrix.org> ie drop transactional and iter and stuff?
<re_irc>
<@dirbaio:matrix.org> no, I mean
<re_irc>
<@dirbaio:matrix.org> currently we do 1 method = 1 trait, so that a readonly spi doesn't impl Write
<re_irc>
<@dirbaio:matrix.org> BUt
<re_irc>
<@dirbaio:matrix.org> an SPI implementing Transfer can read and write, so there's no reason it can't impl Transactional, Read, Write, WriteIter etc
<re_irc>
<@dirbaio:matrix.org> but in practice many HALs implement Write, Trasnfer, but not Transactional
<re_irc>
<@dirbaio:matrix.org> because it's allowed
<re_irc>
<@dirbaio:matrix.org> this means Transactional is almost useless, since drivers can't rely on it being impl'd
<re_irc>
<@dirbaio:matrix.org> so they use Write/Transfer instead
<re_irc>
<@dirbaio:matrix.org> so
<re_irc>
<@dirbaio:matrix.org> instead of doing "1 method = 1 trait", we can do "1 set of hardware capabilities = 1 trait"
<re_irc>
<@firefrommoonlight:matrix.org> Maybe just a `SpiBus` trait?
<re_irc>
<@firefrommoonlight:matrix.org> I'd use that if it could read/write with blocking and DMA
<re_irc>
<@dirbaio:matrix.org> using dma is up to the impls :P
<re_irc>
<@firefrommoonlight:matrix.org> Thus far I've just been using hard-set things, and if I use a diff lib, I'll find+replace `Spi<SPI1>` with `SPIM<SPIM0>` etc
<re_irc>
<@firefrommoonlight:matrix.org> If you're using the trait in a type sig, it needs to have the interface defined or you can't use it
<re_irc>
<@dirbaio:matrix.org> not sure I understand
<re_irc>
<@firefrommoonlight:matrix.org> I guess you could have the impl pick one or the other
<re_irc>
<@firefrommoonlight:matrix.org> Well, I like having `read`, `write`, `read_dma`, and `write_dma` avail etc
<re_irc>
<@dirbaio:matrix.org> all hals should impl the traits, you can write generic drivers over the traits
<re_irc>
<@dirbaio:matrix.org> ah
<re_irc>
<@jbeaurivage:matrix.org> Hey everyone, does anyone have experience with reaching out to the Bosch Sensortec engineering team? I think I've run into a hardware bug, and I'd like to confirm or at least make them aware of it. I've tried contacting them once, but they basically said "go on our forums", which was super unhelpful, because they seem not to want to acknowledge bugs there
<re_irc>
<@dirbaio:matrix.org> the read_dma/write_dma api is very unsafe though, it's unlikely to land on embedded-hal
<re_irc>
<@dirbaio:matrix.org> you could make your own traits
vvlad_ has joined #rust-embedded
<re_irc>
<@dirbaio:matrix.org> the methods would be different though, on stm32 you need to take a channel, and not on nrf
<re_irc>
<@dirbaio:matrix.org> it's hard to abstract dma at that layer :S
vvlad_ has quit [Quit: Leaving]
<re_irc>
<@therealprof:matrix.org> emilgardis:matrix.org: You should be able to do this, too. 😉
<re_irc>
<@emilgardis:matrix.org> I just don't know if the content is correct, but sure ;)
<re_irc>
<@therealprof:matrix.org> Seems accurate to me. We don't do exhaustive fact checking usually. 😉
<re_irc>
<@firefrommoonlight:matrix.org> Hah yea. I was thinking of it since I just de-EHed jam's SSD1331 driver today
<re_irc>
<@firefrommoonlight:matrix.org> So I can use DMA etc
<re_irc>
<@firefrommoonlight:matrix.org> And de-own-SPIed it
<re_irc>
<@firefrommoonlight:matrix.org> nice. Still too traumatized from JS though!
<re_irc>
<@dirbaio:matrix.org> lolol
<re_irc>
<@dirbaio:matrix.org> rust is not JS though
<re_irc>
<@dirbaio:matrix.org> (thankfully!)
<re_irc>
<@firefrommoonlight:matrix.org> I just never really Grokked it. I know Async was/is v popular in both Python and Rust as it was introduced, but I never got into it
<re_irc>
<@firefrommoonlight:matrix.org> Don't really understand it, and never made much of an effort. On the todo list
crabbedhaloablut has quit [Remote host closed the connection]