<re_irc>
<@bradleyharden:matrix.org> > After a contributor has at least one pull request merged into a project's repository, any future pull requests from that contributor's fork will automatically run workflows.
<re_irc>
<@bradleyharden:matrix.org> dirbaio
<re_irc>
<@dirbaio:matrix.org> yeah that's what the doc says, but not what happens in reality 🤣
<re_irc>
<@dirbaio:matrix.org> sometimes I have to approve 2 or 3 times, the same PR even
<re_irc>
<@bradleyharden:matrix.org> merged is the key word there
<re_irc>
<@dirbaio:matrix.org> aaah
<re_irc>
<@bradleyharden:matrix.org> You have to approve all pushes until the first PR is merged
<re_irc>
<@dirbaio:matrix.org> wtf, why
<re_irc>
<@bradleyharden:matrix.org> 🤷♂️
<re_irc>
<@dirbaio:matrix.org> 🤷
<re_irc>
<@adamgreig:matrix.org> are they really worried about someone doing like a trojan horse pr which looks good at first and then they do the crypto switcheroo?
<re_irc>
<@adamgreig:matrix.org> i feel like once you approve it once it's probably 99% going to remain fine...
starblue1 has joined #rust-embedded
fabic has joined #rust-embedded
starblue has quit [Ping timeout: 244 seconds]
rjframe has quit [Ping timeout: 272 seconds]
GenTooMan has quit [Ping timeout: 272 seconds]
GenTooMan has joined #rust-embedded
<re_irc>
<@ryan-summers:matrix.org> @therealprof:matrix.org: I actually believe this to be intentional. It only works to merge if the user doesn't directly push any code (as that code would then be unreviewed). If you have out-of-sync branches on GH, (e.g. based off old master), and you click GH UI to update your branch, the review + merge...
<re_irc>
... will go through in the expected automated fashion
<re_irc>
<@ryan-summers:matrix.org> It's a really useful tool in projects that require strict code review
emerent has quit [Ping timeout: 272 seconds]
emerent_ has joined #rust-embedded
emerent_ is now known as emerent
<re_irc>
<@therealprof:matrix.org> @ryan-summers:matrix.org: Are we talking about the same thing? 😅 The problem we're talking about is that you sometimes have to authorize CI multiple times for the same user. Having to intervent manually means that obvious problems go unnoticed until that happens. Especially with time being scarse at the...
<re_irc>
... moment it's superannoying if you only do reviews every so many days and then you find one, click on authorize and it immediately fails CI.
<re_irc>
<@ryan-summers:matrix.org> Yeah, and what I'm saying is that it can be a useful feature. Github disables automatic autorization when CI passes whenever the user pushes new code
<re_irc>
<@ryan-summers:matrix.org> However, it won't revoke automatic authorization if they use github tools and verified pulls to do basic operations (like rebase onto master first)
<re_irc>
<@ryan-summers:matrix.org> However honestly, they should probably make it configurable
<re_irc>
<@ryan-summers:matrix.org> Unless I'm misunderstanding the covo
<re_irc>
<@therealprof:matrix.org> Yes, that would be my preference, too.
<re_irc>
<@ryan-summers:matrix.org> But yeah, the negative is that once you enable automatic merge, the user could theoretically upload malicious code without you noticing
<re_irc>
<@therealprof:matrix.org> Didn't know about the conditions when they'd auto revoke a previous authorization.
<re_irc>
<@ryan-summers:matrix.org> I don't know if they're specified anywhere, that's just how I've experienced it (I use GH workflows for my day job)
<re_irc>
<@therealprof:matrix.org> Usually people uploading malicious code do not have a very high reputation. It's mostly fake accounts used to do that.
<re_irc>
<@therealprof:matrix.org> Trusting everyone to trusting no-one is quite a jump... Something inbetween would have been nice.
<re_irc>
<@ryan-summers:matrix.org> Yeah, I don't disagree. I think it's tailored more towards the private sector honestly, where companies can enforce all code is reviewed etc. Helps with formal auditing and controlled procedures
<re_irc>
<@ryan-summers:matrix.org> That's honestly why I tend to like it for work
<re_irc>
<@ryan-summers:matrix.org> Something one engineer may think is trivial may actually have impacts
<re_irc>
<@therealprof:matrix.org> Oh for sure.
<re_irc>
<@ryan-summers:matrix.org> However I agree it doesn't make a lot of sense for many crate development projects
fabic has quit [Ping timeout: 244 seconds]
fabic has joined #rust-embedded
neceve has joined #rust-embedded
fabic has quit [Ping timeout: 272 seconds]
Guest3328 has joined #rust-embedded
Guest3328 has quit [Client Quit]
<re_irc>
<@adamgreig:matrix.org> @ryan-summers:matrix.org: I think you're talking about pull request approval (the "approve" review option) in advance of a merge, and therealprof is talking about the very new anti-cryptocurrency-abuse GitHub actions feature that adds a "run CI" button a maintainer must press on a pr from a first-time...
<re_irc>
... contributor to trigger the normal CI run
<re_irc>
<@ryan-summers:matrix.org> Ahhh that makes more sense
<re_irc>
<@adamgreig:matrix.org> Previously CI would/could run automatically as soon as someone opens a PR, but now it must be manually approved for new contributors to stop abuse of GitHub actions minutes
<re_irc>
<@adamgreig:matrix.org> But, the docs suggest you'd only need to press approve once per user, and in fact it's every time a run needs to happen until the user has merged commits in the repo, which means someone's first-time PR is super annoying because a maintainer needs to keep clicking "run CI"
<re_irc>
<@ryan-summers:matrix.org> Ah yeah that would be incredibly frustrating
fabic has joined #rust-embedded
gsalazar has joined #rust-embedded
tokomak has joined #rust-embedded
<re_irc>
<@hanno:braun-odw.eu> @dirbaio:matrix.org: I don't know about `alloc` specifically, but sometimes you need to tell Rust "I'm using this crate, please give it a chance to hook into whatever it hooks into".
<re_irc>
<@hanno:braun-odw.eu> An example of this are panic handlers. You can do `extern crate panic_whatever;`, or `use panic_whatever as _;`, and importing something else from `panic_whatever` will probably work too. But just specifying it in `Cargo.toml` alone is not enough to actually get it included (which makes sense).
<re_irc>
<@hanno:braun-odw.eu> As I said, I don't know about `alloc`, but maybe it's the same there.
<re_irc>
<@dirbaio:matrix.org> yeah, those you can do with `use`
<re_irc>
<@dirbaio:matrix.org> `alloc` is the only place where `extern crate` is really really still needed afaik 🤷♂️
<re_irc>
<@hanno:braun-odw.eu> Weird 😄
<re_irc>
<@dirbaio:matrix.org> very weird
rjframe has joined #rust-embedded
vancz has joined #rust-embedded
tokomak has quit [Read error: Connection reset by peer]
neceve has quit [Ping timeout: 272 seconds]
neceve has joined #rust-embedded
<re_irc>
<@allexoll:matrix.org> i'm having trouble understanding how `svd2rust` decides if an access is unsafe or not, any idea where i can find info on that?
<re_irc>
<@allexoll:matrix.org> i have to write my own SVD for a custom SoC, but i don't think putting `unsafe` blocks everywhere is how it's supposed to be done...
<Lumpio->
As far as I can remember it sets fields where not all bit patterns are valid as "unsafe"
<Lumpio->
I.e. setting the bits of the entire register in many cases, and for fields with enum values, any undefined values
<re_irc>
<@allexoll:matrix.org> uh i see, so pretty normal to have that much `unsafe` i guess
<re_irc>
<@nehalem501:matrix.org> Hello, I'm trying to have some rust code running on a cortex-m0+ MCU, and it doesn't work, could someone help me ? I have tried the exact same code in C and it works.
<re_irc>
<@nehalem501:matrix.org> C code:
<re_irc>
<@nehalem501:matrix.org> int main(void) {
<re_irc>
<@dirbaio:matrix.org> modify does read+modify+write
<re_irc>
<@dirbaio:matrix.org> write does just a write, leaving all unspecified bits at reset values
<re_irc>
<@nehalem501:matrix.org> Isn't that what I'm doing in the C code ? (I'm just writing the bits I'm interested in)
<re_irc>
<@ryan-summers:matrix.org> @allexoll:matrix.org: It just means that whenever you use unsafe, you need to justify why the bit pattern you're programming is a valid setting (e.g. prove it doesnt misconfigure the register to invalid settings)
<re_irc>
<@ryan-summers:matrix.org> @nehalem501:matrix.org: No, you're reading, modifying, and writing
<re_irc>
<@dirbaio:matrix.org> nehalem501: `|=` reads, sets the bit, writes
<re_irc>
<@ryan-summers:matrix.org> The =| syntax is shorcut for value = value | <RHS>
<re_irc>
<@nehalem501:matrix.org> Thanks for the help, by using ```modify``` instead of ```write``` for the initialization part, the LEDs are now switched on
<re_irc>
<@nehalem501:matrix.org> However the blinking part still doesn't work, and I can't use modify on the registers inside the loop it seems.
<re_irc>
<@nehalem501:matrix.org> I may have an answer on why it refuses to compile, according to the manufacturer's documentation this register is write only ("Port Toggle Output Register (GPIOA_PTOR), Access: W (always reads 0))"
<re_irc>
<@dirbaio:matrix.org> ah, you can only write
<re_irc>
<@dirbaio:matrix.org> then do a single write with all the 3 bits set to 1
<re_irc>
<@dirbaio:matrix.org> it's already usable as a separate crate
<re_irc>
<@dirbaio:matrix.org> all the basics (uart, spi, i2c, gpio, exti, etc) are mostly complete, more advanced stuff is added when someone needs it
<re_irc>
<@dirbaio:matrix.org> so there's no "timeline" really
<re_irc>
<@nehalem501:matrix.org> dirbaio: Still doesn't work, I'll have to investigate a bit deeper as all the write-only registers are not working
<re_irc>
<@dirbaio:matrix.org> when you do a write, unspecified bits default to the reset value
<re_irc>
<@dirbaio:matrix.org> which may be 0 or 1
neceve has quit [Ping timeout: 272 seconds]
<re_irc>
<@dirbaio:matrix.org> maybe they're 1, so you're writing 1 .. or 1 which wouldn't be a visible change :D
<re_irc>
<@dirbaio:matrix.org> ah that's a "toggle" register right? then it's probably not that... dunno :S
<re_irc>
<@dirbaio:matrix.org> firefrommoonlight: can release on crates.io if there's interest
<re_irc>
<@firefrommoonlight:matrix.org> Thank you. Do you have an example of how to use it, eg to replace stm32-rs PAC?
<re_irc>
<@firefrommoonlight:matrix.org> And what MCUs it supports?
<re_irc>
<@dirbaio:matrix.org> supports all MCUs, but some will be very empty
<re_irc>
<@firefrommoonlight:matrix.org> Hmm. I think I need to wait until it's complete
<re_irc>
<@firefrommoonlight:matrix.org> Do you know when it would be feasible to replace stm32-rs PACs?
<re_irc>
<@dirbaio:matrix.org> how to use it: just add it as a dep, then you can go and do reg writes like `pac::GPIOA.pupdr().modify(|w| w.set_pupdr(n, val));`
<re_irc>
<@dirbaio:matrix.org> the way development is working is when someone wants a driver in embassy-stm32 for something that's missing in the metapac, they add it to the metapac
<re_irc>
<@dirbaio:matrix.org> so as I said there's no "timeline" to make it complete
<re_irc>
<@firefrommoonlight:matrix.org> Thank you
<re_irc>
<@firefrommoonlight:matrix.org> Looking forward to giving it a shot once rdy
<re_irc>
<@dirbaio:matrix.org> but I'll take PRs that add stuff to the metapac alone
<re_irc>
<@dirbaio:matrix.org> the goal is to make it useful beyond just embassy
<re_irc>
<@dirbaio:matrix.org> but if by "complete" you mean all peris on all mcus, you'll be waiting forever :D
<re_irc>
<@firefrommoonlight:matrix.org> For more context, I'm hitting more and more issues with SVD2RS PACs, and the PR /issue-addressing process seems halted
<re_irc>
<@firefrommoonlight:matrix.org> So my code's getting more and more messy with feature gates, raw reg writes, commenting-out/in various path dependencies in Cargo.toml when I change MCUs etc
<re_irc>
<@firefrommoonlight:matrix.org> I guess I could learn to build the PAC modules from YAMLs, and maintain a separate stm32-hal-PAC, but your metapac sounds like a great approach
<re_irc>
<@dirbaio:matrix.org> if the peri has a `block` key, it's supported
<re_irc>
<@dirbaio:matrix.org> (`block` refers to a yaml in `/data/registers`)
<re_irc>
<@dirbaio:matrix.org> (if it doesn't have a `block`, it'll appear in the pac as `*const ()` until the block is added)
<re_irc>
<@firefrommoonlight:matrix.org> Nice
<re_irc>
<@firefrommoonlight:matrix.org> Ultimately the root cause of this mess is sloppy SVDs. I wish ST would pay an intern or something to QC them before publishing
<re_irc>
<@dirbaio:matrix.org> also, something very cool is the metapac gives you "macrotables" that you can "query" to generate code
<re_irc>
<@dirbaio:matrix.org> for example, this will impl Instance for each usart peripheral
<re_irc>
<@firefrommoonlight:matrix.org> I think we agree
<re_irc>
<@dirbaio:matrix.org> not `Uart<USART3, (PA4<Alternate<AF7>>, PA5<Alternate<AF7>>, PA6<Alternate<AF7>>, PA7<Alternate<AF7>>)>` or whatever, like you sometimes see in the HALs 🤣
<re_irc>
<@dirbaio:matrix.org> but either way, that's up to you as a HAL author
<re_irc>
<@dirbaio:matrix.org> the metapac doesn't do that for you, it just gives you the macrotables so you can setup the types/traits however you want
<re_irc>
<@firefrommoonlight:matrix.org> Yea. That mess is not worth the check
<re_irc>
<@dirbaio:matrix.org> well pins *are* checked, `new` requries the pin impls TxPin/RxPin/etc
<re_irc>
<@dirbaio:matrix.org> but the type is clean :D
<re_irc>
<@firefrommoonlight:matrix.org> It's as if whoever came up with that pattern never tested it in a real project
<re_irc>
<@firefrommoonlight:matrix.org> Or bothered to document it
<re_irc>
<@dirbaio:matrix.org> lol
<re_irc>
<@firefrommoonlight:matrix.org> I like your decision there
fabic has quit [Ping timeout: 268 seconds]
<re_irc>
<@firefrommoonlight:matrix.org> Btw, there is a small but active Zig community working on an embedded infrastructure. It's currently at least 1-2 years from being viable, but there's room for collaboration. For now probably sharing rust patterns like SVD parsing and patching
<re_irc>
<@dirbaio:matrix.org> lol Zig
<re_irc>
<@firefrommoonlight:matrix.org> I think the Rust ecosystem is in a great spot right now, and the issues are more about polishing, and platform support
<re_irc>
<@firefrommoonlight:matrix.org> As well as higher-level frameworks like Embassy and RTIC
<re_irc>
<@firefrommoonlight:matrix.org> Zig still has a ways to go in terms of flashing, debugging, reg access, etc. Not viable currently
<re_irc>
<@firefrommoonlight:matrix.org> Nice lang though
<re_irc>
<@firefrommoonlight:matrix.org> Suitable for embedded IMO
<re_irc>
<@dirbaio:matrix.org> I don't understand the point of Zig
<re_irc>
<@firefrommoonlight:matrix.org> It's a modern C
<re_irc>
<@dirbaio:matrix.org> it's marketed as a "safer C"
<re_irc>
<@dirbaio:matrix.org> and it fixes all the "easy to fix" UBs: overflow, bounds checks
<re_irc>
<@firefrommoonlight:matrix.org> It's also missing package management, which is a showstopper. Should eventually be fixed
<re_irc>
<@firefrommoonlight:matrix.org> Yea - overall, the syntax and experience is nicer. In some ways, it reads more like a subset of Rust
<re_irc>
<@dirbaio:matrix.org> but it leaves the "hard to fix" UBs totally unaddressed: use-after-free, double free, etc
<re_irc>
<@firefrommoonlight:matrix.org> Yep
<re_irc>
<@firefrommoonlight:matrix.org> Not as safe as Rust,band never will be
<re_irc>
<@dirbaio:matrix.org> and these UBs can only be fixed with either GC or borrow checker, so they'll never be fixed in Zig
<re_irc>
<@dirbaio:matrix.org> given that, what's the point of Zig?
<re_irc>
<@firefrommoonlight:matrix.org> A nicer C
<re_irc>
<@dirbaio:matrix.org> "slightly nicer C but still keeping the biggest flaws of C"
<re_irc>
<@adamgreig:matrix.org> but also package management, easy cross compiling, namespace/modules, in principle lots of the other good parts of rust
<re_irc>
<@dirbaio:matrix.org> the "slightly nicer" part isn't worth starting over with a new language imo, throwing away all the ecosystem etc
<re_irc>
<@firefrommoonlight:matrix.org> For example, the appeal of Rust to me isn't the safety as much as the polished tools and experience
<re_irc>
<@adamgreig:matrix.org> maybe that's not enough to make up for not solving some of c's more fundamental unsafety issues
<re_irc>
<@adamgreig:matrix.org> but c's got so many different things to complain about and it fixes a lot of them still, I guess
<re_irc>
<@adamgreig:matrix.org> zig's cross compiling sounds somehow even slicker than rust's
<re_irc>
<@dirbaio:matrix.org> Rust's is ultra slick until you have to link in C code
<re_irc>
<@firefrommoonlight:matrix.org> I'd like to see how it's embedded infrastructure is in a few years
<re_irc>
<@firefrommoonlight:matrix.org> For now, I have no use case for it
<re_irc>
<@dirbaio:matrix.org> I think Zig is slicker because it can compile C itself (?)
<re_irc>
<@dirbaio:matrix.org> but
<re_irc>
<@dirbaio:matrix.org> you might as well do a "Cargo for C" project, with decent package management and crosscompiliation
<re_irc>
<@adamgreig:matrix.org> given zig's ease of compiling and linking to c, it can probably be exactly that too
<re_irc>
<@adamgreig:matrix.org> maybe someone will teach cargo how to compile c as well as zig-cc, who knows
<re_irc>
<@dirbaio:matrix.org> if you're able to fix the crosscompile story for C, you're proving the issue is not the C language, but the tooling
<re_irc>
<@dirbaio:matrix.org> so it's a reason *less* to invent a new language :D
<re_irc>
<@adamgreig:matrix.org> I think there's more than one issue :P
<re_irc>
<@adamgreig:matrix.org> the tooling is bad, the language is also bad
<re_irc>
<@dirbaio:matrix.org> yeah but Zig is still bad
<re_irc>
<@dirbaio:matrix.org> it's "slightly less bad"
<re_irc>
<@dirbaio:matrix.org> still has the big UBs
<re_irc>
<@dirbaio:matrix.org> no traits/interfaces, you gotta build vtables manually like in C
<re_irc>
<@dirbaio:matrix.org> really?
<re_irc>
<@adamgreig:matrix.org> well, luckily you can keep using rust!
<re_irc>
<@adamgreig:matrix.org> zig still seems like a fun exploration of a different set of tradeoffs, anyway
<re_irc>
<@firefrommoonlight:matrix.org> Yea - keep in mind, there are use cases outside of our own cases and preferences
<re_irc>
<@dirbaio:matrix.org> dunno, to each its own I guess
<re_irc>
<@firefrommoonlight:matrix.org> And I think Zig will lead to improvements in Rust and vice versa
<re_irc>
<@adamgreig:matrix.org> @room meeting time again, we'll start in 5min! agenda is at https://hackmd.io/xrN27SU6Qjatm99fm2dkbQ, please add anything you'd like to announce or discuss
<re_irc>
<@adamgreig:matrix.org> announcements first, probably people have noticed but our IRC channel is back on Libera (#rust-embedded), and we have a bridge running so users on each end can see each other
<re_irc>
<@adamgreig:matrix.org> it's not the same bridge as before so we'll see how it goes (and if you're an irc user please give me any feedback)
<re_irc>
<@adamgreig:matrix.org> on the bright side the bridge isn't a room admin so can't kick people, so no more getting kicked from matrix
<re_irc>
<@adamgreig:matrix.org> does anyone have any other announcements?
rjframe has quit [Ping timeout: 244 seconds]
<re_irc>
<@adamgreig:matrix.org> ok, moving on, we're still sort of stuck with cross.. this won't be news to anyone here and I doubt there's anything more to discuss, so just mentioning it as a reminder
<re_irc>
<@therealprof:matrix.org> Where are we with the announcement?
<re_irc>
<@therealprof:matrix.org> I saw some new issue flying by where someone said they really need cross for something.
<re_irc>
<@therealprof:matrix.org> Maybe that's a good chance to finally recruit some future maintainers.
<re_irc>
<@adamgreig:matrix.org> it was reitermarkus who made the previous release iirc
<re_irc>
<@therealprof:matrix.org> Yes, I meant regarding the recruitment.
<re_irc>
<@therealprof:matrix.org> But talking to Markus might also be a first step.
<re_irc>
<@therealprof:matrix.org> Well, step, not first. 😉
<re_irc>
<@adamgreig:matrix.org> 👍️
<re_irc>
<@adamgreig:matrix.org> anyway, let's move on to the HAL stuff
<re_irc>
<@adamgreig:matrix.org> we discussed the two recent PRs last week and didn't really reach a conclusion, but I don't think there was any further discussion on the threads either?
<re_irc>
<@dirbaio:matrix.org> no updates 🤷♂️
<re_irc>
<@dirbaio:matrix.org> guess a decision has to be made
<re_irc>
<@dirbaio:matrix.org> is it ok for blocking/nb method names to conflict y/n
<re_irc>
<@dirbaio:matrix.org> but we already discussed it last wek
<re_irc>
<@eldruin:matrix.org> I think `bwrite_all` is a weird name, no questions there
<re_irc>
<@eldruin:matrix.org> however, it would be nicer if disambiguation would never be necessary
<re_irc>
<@eldruin:matrix.org> Additionally, if we choose the same names, the prelude has to go
<re_irc>
<@adamgreig:matrix.org> my worry is we need to do something now for nb and blocking, but soon also for futures-based traits
<re_irc>
<@eldruin:matrix.org> but I am no user of the prelude, so I do not know how annoying for newcomers that would be
<re_irc>
<@dirbaio:matrix.org> yeah the prelude has to go imo
<re_irc>
<@eldruin:matrix.org> for the record, I am fine with removing the prelude
<re_irc>
<@dirbaio:matrix.org> the prelude doesn't play well with the "HAL defines infallible set_high method AND impls OutputPin" idea either
<re_irc>
<@adamgreig:matrix.org> in the situation where embedded-hal traits are expected to be used primarily by driver authors, not end-user crates, the prelude doesn't seem very useful
<re_irc>
<@eldruin:matrix.org> that with the futures is a good point
<re_irc>
<@dirbaio:matrix.org> because you then get a conflict between the trait and inherent methods
<re_irc>
<@adamgreig:matrix.org> i guess some embedded-hal designs were made assuming end-users would be using the traits directly to interact with their hal, for which the prelude makes more sense
<re_irc>
<@therealprof:matrix.org> Not sure whether there was a big thought behind that.
<re_irc>
<@grantm11235:matrix.org> @adamgreig:matrix.org: In that case I think it makes more sense to put stuff in the hal prelude
<re_irc>
<@adamgreig:matrix.org> yea, maybe it just happened
<re_irc>
<@therealprof:matrix.org> Preludes were much more common a while ago. Maybe they were deemed best practise?
<re_irc>
<@dirbaio:matrix.org> yeah there seems to be a "ALL hardware must have an e-h trait" mentaity
<re_irc>
<@dirbaio:matrix.org> I bet the most obscure e-h traits don't have a single driver using them
<re_irc>
<@adamgreig:matrix.org> right, at one point HALs only exposed things publically via e-h traits
<re_irc>
<@adamgreig:matrix.org> isn't it a condition for a trait to be included that it had at least two implementations? :P
<re_irc>
<@dirbaio:matrix.org> for example the watchdog trait
<re_irc>
<@dirbaio:matrix.org> HALs impl it, users of a particular HAL use it
<re_irc>
<@adamgreig:matrix.org> @grantm11235:matrix.org: right, but these days the expectation is that end users will interact with the HAL using normal non-trait methods, and only generic drivers (or generic application code) need to use the e-h traits to interact with hardware
<re_irc>
<@dirbaio:matrix.org> but why the hell would someone want to write a *driver* generic over any watchdog??
<re_irc>
<@adamgreig:matrix.org> @dirbaio:matrix.org: ah, I see what you mean
<re_irc>
<@adamgreig:matrix.org> having the e-h trait still has value I think: applications can expect HALs to provide the same interface, so it's easy to port between HALs
<re_irc>
<@dirbaio:matrix.org> is there a single driver out there doing `struct Foo<T: Watchdog>`? I bet there isn't
<re_irc>
<@adamgreig:matrix.org> but it's less critical compared to things used by generic drivers
<re_irc>
<@therealprof:matrix.org> I'm also not attached to the prelude.
<re_irc>
<@dirbaio:matrix.org> the problem with "everything has to have an e-h trait" is it leads to a "lowest common denominator" issue
<re_irc>
<@dirbaio:matrix.org> it's just impossible to fit all ADCs into a trait without losing functionality
<re_irc>
<@therealprof:matrix.org> But that's not how traits work.
<re_irc>
<@adamgreig:matrix.org> I don't think anything still insists on "everything has to have an e-h trait, and we'll only expose functionality through it"
<re_irc>
<@dirbaio:matrix.org> so HALs end up impling the trait AND doing a custom feature
<re_irc>
<@eldruin:matrix.org> Hardware is messy, but you can find some common interfaces, and that is useful
<re_irc>
<@adamgreig:matrix.org> sure, but that's always what we expect, right?
<re_irc>
<@adamgreig:matrix.org> even for uarts or whatever
<re_irc>
<@dirbaio:matrix.org> or not even impling the trait sometimes
<re_irc>
<@therealprof:matrix.org> @dirbaio:matrix.org: Which is totally fine.
<re_irc>
<@dirbaio:matrix.org> it is
<re_irc>
<@therealprof:matrix.org> It would also be possible to have multiple traits for different levels of functionality.
<re_irc>
<@dirbaio:matrix.org> but still, the value of more obscure traits is questionable
<re_irc>
<@therealprof:matrix.org> I disagree for the reasons adamgreig mentioned before.
<re_irc>
<@dirbaio:matrix.org> maybe there should be "there has to be at least 1 driver using it" requirement for adding a new trait
<re_irc>
<@therealprof:matrix.org> And I very much also disagree with that.
<re_irc>
<@dirbaio:matrix.org> e-h has traits that are ridiculously hard to use from dirvers because the author clearly only had the HAL side of things in mind
<re_irc>
<@adamgreig:matrix.org> we've diverged somewhat from the two PRs immediately at issue, around basically whether we have overlapping method names
<re_irc>
<@adamgreig:matrix.org> guidelines for adding new traits seems like it can be a somewhat separate issue
<re_irc>
<@dirbaio:matrix.org> the most painful example is IoPin
<re_irc>
<@eldruin:matrix.org> we are open to improvement RFCs
<re_irc>
<@therealprof:matrix.org> I was about to say. ;)
<re_irc>
<@adamgreig:matrix.org> for now at least, it sounds like we could be open to removing the prelude, does that also mean allowing (requiring?) overlapping method names for nb/blocking versions of the same hardware?
<re_irc>
<@therealprof:matrix.org> It would simplify things for sure.
<re_irc>
<@dirbaio:matrix.org> yeah
<re_irc>
<@eldruin:matrix.org> if we will have a futures version and those would need an unique name as well, I guess we will not find any
<re_irc>
<@eldruin:matrix.org> I would have liked unique names, though
<re_irc>
<@dirbaio:matrix.org> you'd go insane with such an API
<re_irc>
<@adamgreig:matrix.org> seems more like just picking synonyms and arbitrarily assigning them to different things
<re_irc>
<@dirbaio:matrix.org> - Remove prelude
<re_irc>
<@dirbaio:matrix.org> - standardize names for the same thing (eg always `read` and `write`)
<re_irc>
<@dirbaio:matrix.org> so, proposal:
<re_irc>
<@dirbaio:matrix.org> - Allow name overlaps between blocking/nb/futures traits
<re_irc>
<@dirbaio:matrix.org> y/n?
<re_irc>
<@nickray:solokeys.com> I think it's idiomatic to use modules to disambiguate (`{blocking,nonblocking,...}::write` etc.), at one call site you'll likely use one of them only, so no confusion.
<re_irc>
<@firefrommoonlight:matrix.org> Y
<re_irc>
<@dirbaio:matrix.org> if it's Officialy Approved ™️ right now I'll PR it
<re_irc>
<@firefrommoonlight:matrix.org> To all
<re_irc>
<@adamgreig:matrix.org> so really 'require name overlaps between blocking/nb/futures, for the same thing'
<re_irc>
<@adamgreig:matrix.org> Nicolas Stalder | SoloKeys: the only downside is you have to use the standard form to disambiguate
<re_irc>
<@adamgreig:matrix.org> e.g. with just `use eh::blocking::write` you can later call `uart.write(data)`, but if you need to use both, it's `eh::blocking::write::write(&mut uart, data)` or whatever
<re_irc>
<@nickray:solokeys.com> what standard form? you can import however you like in any case (import nb::write as nb_write)
<re_irc>
<@adamgreig:matrix.org> you can't use methods from inside traits
<re_irc>
<@therealprof:matrix.org> @dirbaio:matrix.org: Well, the process is the other way around... Approval happens on the RFC. 😉
<re_irc>
<@adamgreig:matrix.org> you can use the trait so it's shorter, at least
<re_irc>
<@firefrommoonlight:matrix.org> Hmmm... I may go with 'write_one' for single byte Spi write, and 'write' for multi-byte blocking
<re_irc>
<@dirbaio:matrix.org> @therealprof:matrix.org: then go LGTM the PRs :P
<re_irc>
<@firefrommoonlight:matrix.org> And wrap with whatever EH API you settle on for generic drivers
<re_irc>
<@dirbaio:matrix.org> but I'm referring to approving the "policy", the PRs are particular instances of it
<re_irc>
<@therealprof:matrix.org> Not all team members are present so no one to approach the "policy".
<re_irc>
<@dirbaio:matrix.org> but there's 2 out of 3, and ryankurte has already lgtm'd one PR
<re_irc>
<@eldruin:matrix.org> I know @ryankurte would have also liked to have unique names, and it will be a bit problematic for `embedded-hal-compat`
<re_irc>
<@eldruin:matrix.org> but anyway, the current approach LGTM
<re_irc>
<@eldruin:matrix.org> still, the PRs are the right way
<re_irc>
<@therealprof:matrix.org> dirbaio: If you update the issue with our discussion here, I'll be happy to sign off on it.
<re_irc>
<@adamgreig:matrix.org> (we'd need a new PR to actually do the renames, right?)
<re_irc>
<@eldruin:matrix.org> we definitely need one to remove the prelude
<re_irc>
<@eldruin:matrix.org> but I can do that
<re_irc>
<@firefrommoonlight:matrix.org> OK, here's the native SPI write API I'm using: `write_one` for NB write one word. `write` for blocking multiple bytes. `write_dma` for multiple bytes DMA
<re_irc>
<@firefrommoonlight:matrix.org> Note that EH I2C's `write` is multiple bytes
<re_irc>
<@dirbaio:matrix.org> `write`, `write_dma` would both be `write` in the trait, the fact that it uses DMA or not is an implementation detail
<re_irc>
<@adamgreig:matrix.org> how would write_one be non-blocking, too?
<re_irc>
<@adamgreig:matrix.org> easy enough to pass &[byte] in to a multi-byte method, right?
<re_irc>
<@firefrommoonlight:matrix.org> Perhaps you could make it generic like that, but the `write_dma` I'm using has several dditional args:
<re_irc>
<@firefrommoonlight:matrix.org> So, not compatible with a single trait, and `write_dma` as I'm doing it is probably fundmamentally not EH compat
<re_irc>
<@adamgreig:matrix.org> lol, I love the bridge bot's little ✂️
<re_irc>
<@adamgreig:matrix.org> yea, fair enough, that's just some extra thing your hal does
<re_irc>
<@nickray:solokeys.com> you might set defaults for those additional parameters, and prepend your `write` with a builder method that passes them to override
<re_irc>
<@firefrommoonlight:matrix.org> edit: That was I2C impl fixed
<re_irc>
<@nickray:solokeys.com> so the "built" writer implements the simpler HAL write
<re_irc>
<@dirbaio:matrix.org> does it block until DMA is done? oherwise it should be `unsafe`
<re_irc>
<@firefrommoonlight:matrix.org> No. Queues the DMA transfer, and user has to catch the stop with an interrupt or poll in user code
<re_irc>
<@adamgreig:matrix.org> what if the buffer goes away?
<re_irc>
<@adamgreig:matrix.org> "don't do that" -> the method call should probably be `unsafe`?
<re_irc>
<@adamgreig:matrix.org> anyway, sorry, getting off topic a bit
<re_irc>
<@firefrommoonlight:matrix.org> Bad things happen if the buffer goes away
<re_irc>
<@firefrommoonlight:matrix.org> Good call - I'll change that
<re_irc>
<@adamgreig:matrix.org> thanks for the E-H discussion everyone, glad to have moved it forward a bit
<re_irc>
<@dirbaio:matrix.org> back to the e-h stuff: what next? HAL team approves+merges PRs?
<re_irc>
<@dirbaio:matrix.org> these are the only renames, there are no others
<re_irc>
<@adamgreig:matrix.org> I guess an option is to merge both of those PRs, add a new PR removing the prelude
<re_irc>
<@adamgreig:matrix.org> nothing else needs to change to make things consistent between nb and b?
<re_irc>
<@dirbaio:matrix.org> nothing :P
<re_irc>
<@dirbaio:matrix.org> there's not much overlap anyway
<re_irc>
<@dirbaio:matrix.org> most traits are only blocking or only nb
<re_irc>
<@adamgreig:matrix.org> could you add a comment to the PRs just summarising the conclusions here and then the hal team can approve/merge?
<re_irc>
<@adamgreig:matrix.org> great, that's it for this week's meeting then, thanks all!
<re_irc>
<@grantm11235:matrix.org> Why does serial have `flush` but spi and i2c don't?
<re_irc>
<@dirbaio:matrix.org> good question!
<re_irc>
<@dirbaio:matrix.org> for i2c it probably doesn't make sense since it waits until the transfer is fully done to check for NAKs etc
<re_irc>
<@adamgreig:matrix.org> this sort of problem has caused some issues with spi where people de-assert cs before the last byte is all written out because it's not flushed, hm
<re_irc>
<@adamgreig:matrix.org> though i think in those cases it's more an oddity of stm32's spi timing
<re_irc>
<@dirbaio:matrix.org> it depends on the hw, a naive spi impl on the RPi Pico would also have that issue
<re_irc>
<@dirbaio:matrix.org> so yeah adding flush would be a very good idea imo
<re_irc>
<@adamgreig:matrix.org> yea, so wonder if blocking::write should just be specified to always flush before returning
<re_irc>
<@adamgreig:matrix.org> or add flush to spi, sure
<re_irc>
<@dirbaio:matrix.org> specifying that it has to wait would make stuff slower
<re_irc>
<@adamgreig:matrix.org> maybe nicer to have the latter since returning once it's in the fifo but before it's finished writing sure makes it a lot easier to do gapless writes
<re_irc>
<@grantm11235:matrix.org> What happens if you don't flush? Will the data still be sent?
<re_irc>
<@adamgreig:matrix.org> so it's "block until all data has been accepted by the hardware for transmission"
<re_irc>
<@dirbaio:matrix.org> if you don't flush it's unknown what happens
<re_irc>
<@dirbaio:matrix.org> for example if you write a word then immediately drop the Spi driver, the word might get cut off
<re_irc>
<@therealprof:matrix.org> @adamgreig:matrix.org: Well, actually that's a bit underdefined.
<re_irc>
<@grantm11235:matrix.org> I think it would make more sense if `write` always blocked until all the data was sent, but that would be inefficient for lots of small writes
<re_irc>
<@adamgreig:matrix.org> therealprof: I mean, perhaps that's what it should be defined as
<re_irc>
<@adamgreig:matrix.org> but yea, I guess it's possible if you drop the spi driver then the hal might disable it, stopping transmission
<re_irc>
<@adamgreig:matrix.org> so it's not a guarantee that the data _will_ be sent
<re_irc>
<@adamgreig:matrix.org> just that it's also not guaranteed that the data has already been sent, only that it's been accepted by the hardware for transmission (i.e. the provided memory is no longer required)
<re_irc>
<@dirbaio:matrix.org> yeah it should be defined in *some way*
<re_irc>
<@adamgreig:matrix.org> like, it's a _blocking_ interface, so we should know what it's meant to block on
<re_irc>
<@dirbaio:matrix.org> otherwise you get hal+driver combos that don't work
<re_irc>
<@therealprof:matrix.org> I think it should be blocking until the signal hit the wire.
<re_irc>
<@adamgreig:matrix.org> which signal, though?
<re_irc>
<@adamgreig:matrix.org> until the entire thing has been written and the transmitter is idle again?
<re_irc>
<@adamgreig:matrix.org> that's simplest (you don't need a flush at all) but very inefficient and makes it impossible to do gapless transmission between multiple calls, which seems maybe unnecessarily wasteful
<re_irc>
<@adamgreig:matrix.org> I guess it should always be possible to guarantee because that's hopefully what flush() would require, though
<re_irc>
<@dirbaio:matrix.org> from time to time people come here asking "why isn't my SPI gapless", so people do care about that
<re_irc>
<@therealprof:matrix.org> People with soft-CS complained that data was buffered in the internal buffer and the blocking called returned.
<re_irc>
<@adamgreig:matrix.org> especially when they're using spi to drive smart leds, lol
<re_irc>
<@grantm11235:matrix.org> @adamgreig:matrix.org: This could also be fixed by making the `drop` impl block until all the pending data is sent
<re_irc>
<@adamgreig:matrix.org> GrantM11235: yes, good point, drop should totally call flush internally
<re_irc>
<@dirbaio:matrix.org> um if you're dropping it it means you no longer care about it
<re_irc>
<@dirbaio:matrix.org> if you do care about it, call flush before dropping
<re_irc>
<@dirbaio:matrix.org> drop shouldn't autoflush imo
<re_irc>
<@adamgreig:matrix.org> if your rust program terminates but it hasn't flushed your "hello world" println to stdout, you don't want it to just stop halfway through either, though
<re_irc>
<@dirbaio:matrix.org> also blocking in drops sucks for async
<re_irc>
<@dirbaio:matrix.org> what does it mean for a firmware to "terminate"?
<re_irc>
<@adamgreig:matrix.org> I was just using it as an example where you'd expect a buffer to be flushed even though you've dropped it
<re_irc>
<@dirbaio:matrix.org> you can "terminate" with SCB::sys_reset(), but then drops won't run
<re_irc>
<@adamgreig:matrix.org> @dirbaio:matrix.org: "if you want it disabled, call disable before dropping"
<re_irc>
<@dirbaio:matrix.org> also autoflushing on drop is extra bloat that the user can't remove if they don't care, or they have already fulshed
<re_irc>
<@adamgreig:matrix.org> yea, all fair points, perhaps dropping shouldn't touch the hardware at all
<re_irc>
<@adamgreig:matrix.org> disabling the peripheral on drop is also extra bloat I can't remove
<re_irc>
<@dirbaio:matrix.org> embassy disables but doesn't flush on drop
<re_irc>
<@dirbaio:matrix.org> lol
<re_irc>
<@dirbaio:matrix.org> I see your point 🤣
<re_irc>
<@adamgreig:matrix.org> I know, hence my example :P
<re_irc>
<@TimSmall:matrix.org> The Principle of Least Astonishment should apply I think - but there may be disagreements about what that would be.
<re_irc>
<@dirbaio:matrix.org> if you're dropping a driver, it's because you want to recreate it later
<re_irc>
<@adamgreig:matrix.org> yea, in this case it's not clear to me what's least surprising, maybe any action taken on drop is too surprising
<re_irc>
<@dirbaio:matrix.org> in which case you want it disabled 100% sure
<re_irc>
<@adamgreig:matrix.org> is it?
<re_irc>
<@grantm11235:matrix.org> What if there was a method that took a closure and always flushed after the end of the closure?
<re_irc>
<@adamgreig:matrix.org> GrantM11235: seems like it would be just as easy to forget to use this closure-using-method as to forget to call flush
<re_irc>
<@grantm11235:matrix.org> I mean the api would be `write` which always flushes, and the closure-based thing which also always flushes (and no explicit flush)
<re_irc>
<@adamgreig:matrix.org> so you need a closure for every time you write anything?
<re_irc>
<@adamgreig:matrix.org> oh, sorry
<re_irc>
<@adamgreig:matrix.org> I see
<re_irc>
<@adamgreig:matrix.org> not sure it would really help with the use case we're trying to protect around gapless playback
<re_irc>
<@adamgreig:matrix.org> hmmm
<re_irc>
<@adamgreig:matrix.org> or might make it awkward, anyway?
<re_irc>
<@dirbaio:matrix.org> the closure api is awkward
<re_irc>
<@dirbaio:matrix.org> you wouldn't be able to .await inside it
<re_irc>
<@grantm11235:matrix.org> @dirbaio:matrix.org: Would you want to do that with a blocking serial api?
<re_irc>
<@dirbaio:matrix.org> the arguments are pretty much what's already discussed in that thread. If someone feels something is missing please add it
<re_irc>
<@dirbaio:matrix.org> also posted PR removing prelude
<re_irc>
<@come_744:converser.eu> For flush and disable, I think there is a difference : maybe it is expected that the peripheral be disabled on drop because it is enabled on construction I guess. But for a flush on drop it is not clearly the opposite of the construction so might be surprising. Also I guess disabling is much faster than...