Change DMA API to use embedded-dma traits

[thalesfragoso]

Fixes #270

This PR is pretty much done, but it would be nice to test at least the affected examples on hardware. Unfortunately, I don't have the time to do it right now, I would appreciate some help.

CC @TheZoq2 and @mitchmindtree

[TheZoq2]

Awesome, i'll have a look at it tomorrow :)

[TheZoq2]

Looked through the code and added a few nitpicks/questions.

I'll do some testing of the examples now

[TheZoq2]

I'm probably missing something here, but why is this required? It seems like this would be the smae as just moving self.buffer and self.payload into the tuple directly?

[thalesfragoso]

Transfer now implements Drop, so we can't move out of its fields.

[TheZoq2]

Ah right. Could you add a comment clarifying that in the code? Seems like something that will come up in the future

[TheZoq2]

Alright, tested the serial TX and spi TX examples and they still work. The rx examples are a bit more tricky, but I'll try to get something working

[thalesfragoso]

@TheZoq2 Alright, I'm marking this as ready for review, but if you could test the circular adc example (adding some prints to check it), it would be great.

[TheZoq2]

Finally got around to testing the circular ADC DMA. Looks like things work as expected 🎉

[TheZoq2]

I guess the question that remains is what we should do with the previous versions.

Given the severity, it might make sense to yank the affected version. According to crates.io, we still have plenty of people on 0.6, 0.5 and older versions, and we have quite a few breaking changes lined up.

Would it make sense to yank the latest versions of 0.4, 0.5 and 0.6, then release a new version of them with the DMA things marked as deprecated? That way people who don't use DMA can avoid upgrading right now, while people who do get notified about the problem.

[therealprof]

Hm, the yanking mechanics is subtle. I'm not sure it would have the intended effect if we did that. But I'll leave that decision up to you.

[TheZoq2]

In what way is it subtle. I have not dealt much with yanking so I may be missing something here.

As I understand it the effect will be that people who clean build their project will either get bumped to the patched version and see the deprecation warning, or get a message about the version they are using (in the lockfile or Cargo.toml) being yanked. Obviously we can't do much for people who don't do a clean build

[thalesfragoso]

then release a new version of them with the DMA things marked as deprecated

I don't think deprecation conveys the right idea here, so not sure if it's worth it, and I guess people can still use yanked versions without problems (if they want to), no ?

[therealprof]

In what way is it subtle. I have not dealt much with yanking so I may be missing something here.

Me neither but the exact workings are a mystery to pretty much everyone which is why it is subtle.

As I understand it the effect will be that people who clean build their project will either get bumped to the patched version and see the deprecation warning, or get a message about the version they are using (in the lockfile or Cargo.toml) being yanked. Obviously we can't do much for people who don't do a clean build

My understanding is that it pretty much only works for clean builds without Cargo.lock. If you have a Cargo.lock it'll continue working, if you don't then I think it depends on what the and previous versions are and what is specified in the Cargo.toml: If you have a minor specified and there's a minor update it'll fall forward, if the minor version is the last one in row (which is typical if you have breaking changes due to ) it'll fall back to the previous patch version (which typically has the same problem). To upgrade the minor you need to change the Cargo.toml.

[TheZoq2]

I don't think deprecation conveys the right idea here, so not sure if it's worth it, and I guess people can still use yanked versions without problems (if they want to), no ?

I'm not sure, I thought yanked versions are not distributed anymore? As for conveying the right idea, I figure a correctly worded deprecation message should give roughly the right idea.

My understanding is that it pretty much only works for clean builds without Cargo.lock.

Yea, i'm not sure how cargo.lock will affect things with yanked versions.

and there's a minor update it'll fall forward, if the minor version is the last one in row (which is typical if you have breaking changes due to ) it'll fall back to the previous patch version

This will not be a problem if we release this new version with deprecated DMA though.

To upgrade the minor you need to change the Cargo.toml.
I don't believe so, unless you have explicitly requested a specific version. The default of
x.y.z actually x.y.z^ as I understand it.

[therealprof]

x.y.z actually x.y.z^ as I understand it.

You probably mean ^x.y.z.

But minor level means y, not z which is the patch level. Below 1.0 the Rust compatibility rules say that only the patch level may automatically change to a higher version. Unfortunately tools like cargo add always add a patch version so you can't even fall back to a previous patch version (if such a version would exist, that is).

[TheZoq2]

Oh right, I may be getting things confused with the patch vs minor version thing then since I'm so used to 0.x crates

What I'm suggesting is that we release a

0.7.0: with fixed DMA
0.6.2: 0.6.1 with DMA deprecated
0.5.3: 0.5.2 with DMA deprecated

As I understand it, that would automatically bump clean builds of 0.6.x and 0.5.x users to a non-breaking version with a deprecation message, while new users would get the 0.7 version

[TheZoq2]

Alright, 0.7 has been released 🎉