Additional ioctl

[fpagliughi]

Has anyone implemented any additional ioctl() calls? Particularly I2C_RDWR? If not I will give it a try.

[RandomInsano]

I'm definitely interested in I2C_RDWR, but to make it space/computationally efficient is going to be difficult...

[RandomInsano]

My current plan for the API is a factory method pattern. Here's how an external user would interact with it:

const ADDRESS1 = 0x52;
const ADDERSS2 = 0x62;
const MESSAGE: &'static [u8] = &[0x01, 0x32, 0x04];
let data1 = [0u8; 1];
let data2 = [0u8; 12]

let builder = I2C_Messages::new(ADDRESS1);

let mut items = [
    builder::write(&MESSAGE),
    builder::read(&data),
    builder::custom(&data, ADDERSS2, I2C_M_IGNORE_NAK),
];

let i2cdev = LinuxI2CDevice::new(device, ADDERSS1).unwrap();
i2cdev.transfer(&items);

I spent some time reading over the source code and have a few thoughts that stand out:

1. Slave addresses as specified in the builder and the LinuxI2CDevice creation is superfluous. Should the factory be created from i2cdev?
2. I have no idea how to make a generic factory (in core.rs) that can be extended (in linux.rs) but I'll figure that out. FreeBSD has a very similar interface so I can make a freebsd.rs as well.
3. We'll have to write good examples for both. I don't think there's any risk in mixing and matching, but it can be confusing to know which to use and when.
4. There will need to be a new error type to handle I2C_RDRW-specific edge cases like exceeding the message length or the number of messages as well as bad Linux device versus bad slave address.
5. This feels a little bolted on and very little code from one communication method is needed for the other.

Specifically for points 3, 4, and 5 I wonder if a new crate should be created. I don't like fragmentation as it divides contributors but I can't see a case where you would need both methods at the same time. I'm honestly good with either option, but am leaning towards a separate crate.

[RandomInsano]

Proposed working solution without abstraction, good error handling, or using i2cdev yet is in this snapshot.

[piersfinlayson]

I have a pretty complete implementation and a working example here.

[RandomInsano]

Ooo! I have one sitting somewhat unfinished here that I wanted to polish up: https://github.com/rust-embedded/rust-i2cdev/pull/45/files

I like that you went with a different trait for this. It didn’t feel right to bolt it into the existing one, but I wanted to finish it up before getting advice.

[piersfinlayson]

I went with a different trait because the I2C_RDWR isn't associated with a particular I2C slave device (which is what LinuxI2CDevice represents), but rather operates at the bus level instead.

I also went with Vectors for the messages and the data buffers so that they could be dynamically sized by the application at runtime, rather than using rust's compile time sized arrays.

[eldruin]

We encountered a problem with the current implementation of WriteRead in linux-embedded-hal as the write and read are independent and this is refused by the VEML6030 device. (See: https://github.com/rust-embedded/linux-embedded-hal/issues/25 for more info) I had a look at @RandomInsano's and @piersfinlayson's and formulated a new proposal in #50

[RandomInsano]

That PR looks good to me!

Regards,

Edwin

On Nov 7, 2019, at 2:42 PM, Diego Barrios Romero notifications@github.com wrote:

 We encountered a problem with the current implementation of WriteRead in linux-embedded-hal as the write and read are independent and this is refused by the VEML6030 device. (See: rust-embedded/linux-embedded-hal#25 for more info) I had a look at @RandomInsano's and @piersfinlayson's and formulated a new proposal in #50

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[eldruin]

Alternatively, I submitted #51, which goes further and regains the ease of the original #45 for an existing I2CDevice.

[posborne]

Closing, thank you @eldruin and @RandomInsano for pushing this along, sorry for the delays along the way.