I2C NACK error nesting

[yodaldevoid]

This idea was first proposed here, but broken out to keep things moving.

This merges ErrorKind::NoAcknowledgeData and ErrorKind::NoAcknowledgeAddress into one variant with a source field to indicate what the NACK was in response to. Some drivers may not be able to differentiate between NACKs on address or data bytes so they should use NoAcknowledgeSource::Unknown.

Feel free to bikeshed names and documentation. I used the most obvious names to me, but naming is one of the hardest problems in programming.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ryankurte (or someone else) soon.

Please see the contribution instructions for more information.

[adamgreig]

Thanks for the PR!

Do you have any examples of an implementation that can't distinguish address vs data NAKs? I understand they could exist but I couldn't find any, so I wonder if they do exist in practice.

[yodaldevoid]

Do you have any examples of an implementation that can't distinguish address vs data NAKs? I understand they could exist but I couldn't find any, so I wonder if they do exist in practice.

Based off of this comment most (if not all) HALs currently don't differentiate between NACKs on data vs. address.

As a quick check, I looked at the STM32L0x3 reference manual and based on the possible interrupts it does not differentiate between NACKs on address and data. I suppose you could figure out where the NACK occurred based on if you had previously seen the address match interrupt. This sort of decoding might be possible in other HALs, and if I have time later I will dig through what I can find.

Beyond that, where I work we have an internal device that does not differentiate between NACKs on address vs. data. Sadly, because it is an internal device I can't provide any documentation. I would also not disagree with anyone who says that that device is badly designed to give so little feedback.

[adamgreig]

Thanks for the feedback! It's especially useful to hear about custom devices as that's of course exactly the sort of thing we can't find by searching around.

As a quick check, I looked at the STM32L0x3 reference manual and based on the possible interrupts it does not differentiate between NACKs on address and data. I suppose you could figure out where the NACK occurred based on if you had previously seen the address match interrupt. This sort of decoding might be possible in other HALs, and if I have time later I will dig through what I can find.

This is more or less what I'd expect - the device says it saw a "NAK", but to follow the I2C state machine you would know if it was in the address or data phase. For example in stm32f4xx-hal, a NACK here always means an address NACK (because we're waiting for the ADDR bit to be set, indicating the address has been sent and ACK'd), while a NACK in send_byte always means a data NACK (because the address has already been ACKd in this transaction).

Does the implementation for your custom device not permit anything similar? Is the interface a combined address+data and you just get back a single transaction status, or do you send an address, get a result, send some data, get a result, etc?

[yodaldevoid]

Does the implementation for your custom device not permit anything similar? Is the interface a combined address+data and you just get back a single transaction status, or do you send an address, get a result, send some data, get a result, etc?

The device we have will indicate if a NACK has occurred in the transaction, but not where. This is true both for "raw" I2C reads and writes in addition to the defacto register read and write scheme that many devices implement. If we are reading data and we have received some of our requested data we can detect a data NACK in that case, but that is it. This is an older API that we are still trying to support so we can't easily add in differentiated NACKs.

[Sh3Rm4n]

I'd like to make the case, that this might improve ergonomics besides the case, if NoAcknowledge(NoAcknowledgeSource::Unknown) needs to exist because there are cases where it can not differentiated:

1. I can imagine scenarios, where it's only important, that any transmission was NAKed. I mean, NAK is a defined signal in the I2C protocol, indepent of the case, if it's generated after the address or data line.
2. In that case, it's as easy to match on any NAK with NoAcknowledge(_) => ... instead of NoAcknowledgeAddress | NoAcknowledgeData. You could even bind the source to a name, so that levels further down could handle the different NAK cases.
3. Without that PR it is impossible to go the neutral route. Imagine the case, where someone wants to implement an abitrary I2C driver in a constraint non ideal environment. Currently they have no choice but possibly go through hoops to decide / find out if the NAK was a data or address NAK. With this PR they have the possibility to just return a NAK with source Unkown and possibly reducing the complexity of the driver implementation`

[therealprof]

LGTM

[therealprof]

bors r+

[bors]

Build succeeded:

• ci-linux (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt