ESP32 I2C: Rename ACK_ERR to NACK

[bugadani]

Allows cleaning up things like https://github.com/esp-rs/esp-hal/blob/main/esp-hal/src/i2c.rs#L650-L653

[jessebraham]

Thanks, LGTM!

[playfulFence]

W8, it does clean up things, but then there's no consistency with TRM nor IDF anymore.
Over time it will become really-really tedious to find needed field. Are we sure we need to do that?

[bugadani]

Indeed; i think we agreed that we accept this tradeoff. The documentation still reads as "The masked interrupt status for ack_err_int interrupt" so I think it's obvious wihch bit the nack function refers to. It's a bit harder to find nack based on the TRM, but I think that's not that big a price to pay.

[playfulFence]

Sorry, but I don't really agree with the statement that it's not that big price to pay 😅.

I mean, after some amount of such "simplifications" it potentially might (and it will) become really expensive to find necessary field/register in terms of time. That is why I think that a small #[cfg...] is actually a smaller price to pay.

As an alternative, we might add something like "PREVIOUS NAME:" or "ORIGINAL NAME:" for such fields/registers just so it's possible to find that at least somehow fast

[MabezDev]

@playfulFence I do agree with the principal of your message, that we should be careful when renaming to only do it where it makes sense.

I see cfg_if and cfg as a necessary evil, they're useful and sometimes needed, but in general they make the code harder to read to avoiding them where possible is a good idea. I believe in this case the change is justified, all chips except the esp32 use the nack terminology so in this instance it makes sense to be consistent across all chips, at the cost of not having direct CTRL+F capability in the esp32 trm.

[MabezDev]

As an alternative, we might add something like "PREVIOUS NAME:" or "ORIGINAL NAME:" for such fields/registers just so it's possible to find that at least somehow fast

Regarding this, were you suggesting this in a doc comment or have two methods pointing to the same register?

[playfulFence]

@MabezDev
At least a doc comment yes, just to be able to (somehow) quickly find a field/register by its original aka. TRM/IDF-compatible name.

For some reason I feel that having two methods pointing to the same register might be even more frustrating

[bugadani]

Maybe we could solve this at the svdtools level, so it could generate a (Renamed from XYZ) at the end? (May not be one of the best of ideas)