WMT-GmbH / pn532

no_std implementation of the Pn532 protocol using embedded_hal traits
Apache License 2.0
18 stars 16 forks source link

EOPNOTSUPP using raspberry pi I2C interface #19

Open SirVer opened 3 weeks ago

SirVer commented 3 weeks ago

I am playing around with an PN532 NFC HAT on a RaspberryPi 3B using I2C. I got everything to compile, but always received EOPNOTSUPP when trying to do anything with the device. The linux example code worked fine.

After some debugging I narrowed the problem down to this line: https://github.com/WMT-GmbH/pn532/blob/25a0e4dc7f21dd59786a96436b5becdc95568223/src/i2c.rs#L64

For some reason, the raspberry i2c (or the implementation from linux-embedded-hal?) does not like the two Read Commands. I changed the code to this:

    fn read(&mut self, buf: &mut [u8]) -> Result<(), Self::Error> {
        let mut kbuf = vec![0; buf.len() + 1];
        let res = self.i2c.exec(
            I2C_ADDRESS,
            &mut [Operation::Read(&mut kbuf)],
        );
        buf.copy_from_slice(&kbuf[1..]);
        res
    }

And with this, everything works wonderfully. The implementation is somewhat ugly because of the temporary buffer and the copying over, but I did not find another way. That said, I am not very experienced with I2C, so maybe I am doing it wrong?

I am happy to provide a merge request if the above solution is acceptable to you.

dimpolo commented 3 weeks ago

Hi, thanks for writing and taking the time to analyze this.

From looking at the linux embedded hal crate this could be caused by https://github.com/rust-embedded/linux-embedded-hal/issues/82

Unfortunately we can't use your fix, since vec! won't be available on no_std targets.

Maybe we could look into resolving the linux driver issue instead

SirVer commented 3 weeks ago

While I in principal agree with you that the fix in the linked repo would be right place to fix this. However, this is above my technical abilities and I still need a working solution for my project. So my suggestion is to do a workaround for a moment in this repo. I see two paths:

  1. Extend the static buffer by one byte and read into this, then drop the first byte when doing anything with the data. This is a bit cumbersome in the code right now, but I think could be made to work.
  2. Use conditional compilation (i.e. if no-std is on, use the current code, otherwise use my suggested implementation). I think this is the easiest workaround for now and should not cause further issues, because linux-embedded kinda implies that std is available.

What is your preferred solution? I can try to provide a patch.

dimpolo commented 2 weeks ago

Looking further into it there seems to be a fix ready: https://github.com/rust-embedded/rust-i2cdev/pull/85

Maybe you could ask the devs of rust-i2cdev and linux-embedded-hal to publish a new release.

That being said, if you'd like I would accept a PR with your fix gated behind a linux-i2c-compat feature. (that feature would also enable the std feature)

Note that we'd probably remove that fix in a future release, when the upstream crates published new releases