Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C API should support Restart/Repeat condition #129

Closed
shaggygi opened this issue Dec 31, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@shaggygi
Copy link
Collaborator

commented Dec 31, 2018

It appears you currently can only read and write using I2C API where it provides a Start/Stop at beginning and end of command. Many I2C components will work by writing the address with a Write command and then follow with a ReadByte() or Read() for multiples. However, there are components that require a Reset condition between writing and reading.

@bigdnf mentioned this in #111

Windows.Devices.I2c appears to have related support with a WriteRead():
https://docs.microsoft.com/en-us/uwp/api/windows.devices.i2c.i2cdevice.writeread#Windows_Devices_I2c_I2cDevice_WriteRead_System_Byte___System_Byte___

It appears the UnixI2cDevice has some started work here. Have not found anything public to use similar to SPI TransferFullDuplex.

Couple Linux references

@shaggygi shaggygi changed the title [Proposal] I2C API should support Restart/Repeat condition I2C API should support Restart/Repeat condition Dec 31, 2018

@shaggygi

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 12, 2019

I was tinkering with this for a little bit and was able to get the Restart bit to occur between write/read buffers. I'm just not so sure on the best solution as devices can required different sequences of this. For example, You might have to write a few bytes for addressing, followed by a Restart bit, write another for a command and then remaining data can be clocked in to read buffer. One question is how can this be structured to allow multiple types of bits in specific locations of command? 🤔

I'm putting these notes here to ponder later.

Added to I2cDevice.cs:
public abstract void WriteRead(Span<byte> writeBuffer, Span<byte> readBuffer);

Added to UnixI2cDevice.Linux.cs:

public override unsafe void WriteRead(Span<byte> writeBuffer, Span<byte> readBuffer)
{
  Initialize();

  fixed (byte* writeBufferPointer = writeBuffer)
  {
    fixed (byte* readBufferPointer = readBuffer)
    {
      Transfer(writeBufferPointer, readBufferPointer, writeBuffer.Length, readBuffer.Length);
    }
  }
}
@joperezr

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

I think we can add the WriteRead method, but individual device bindings should still have to think about the specific protocol they require (meaining the Restart bit that you mention above) since that can't really be generalized. I would expect each binding to have all of that logic in their source code, and then call either Write(), Read() or WriteRead() methods as appropriate.

@joperezr

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

Next steps for this: We would need a formal Api proposal for this in order to consider adding it to the library, so feel free to submit one.

@shaggygi

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2019

Just to note, all the bindings I've worked with appear to work by setting register pointer using a Write first and then follow up with the Reads. So it hasn't hindered me so far. Most of the device datasheets have diagrams on the steps to perform where a few had some more complex steps to do things like sequential writes/reads (and for more than one cycle of Writes/Reads within one command).

I also came across a note in a random blog that stated they had devices that would only work using the Reset condition and extra steps to configure the RPi. They were not using .NET IoT, didn't provide the device names, so I couldn't confirm their accuracy.

I was able to produce the Reset condition with above code, but needs to be thought through more. I'm okay with keeping this issue open for a period to allow others to chime in and keep record on possible devices that require this scenario. Investigate further if popular or close if not.

Thx

@ZhangGaoxing

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

Hope to be implemented in the master branch as soon as possible.
I have encountered problems in adding KT0803L binding recently. This is not the only one. When I used the old version System.Devices.Gpio package, it contained ReadWrite(), and MLX90614 worked well. However, Write()Read() mode does not work correctly when using the current System.Device.Gpio package.
@shaggygi @joperezr @krwq

@joperezr

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

Adding WriteRead sounds reasonable to me. @ZhangGaoxing do you want to put up a PR for it?

@ZhangGaoxing

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@joperezr Let you do it. I'm not familiar with Linux drivers.😀

@krwq

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

@ZhangGaoxing @shaggygi - @joperezr is currently on vacation so won't be able to help for couple of weeks. I think this just uses just some linux APIs I can help with linux implementation if it's not too much work although I'm not familiar with that piece of code - I don't think I currently have any devices which require Restart/Repeat so will be hard to test if it is working - perhaps one of you could start WIP PR and ask questions if you're not sure about something - or alternatively please point me to PRs which are broken because of lack of that and I can try to fix but you will need to tell me if it work...

add two work incorrectly sequence diagrams

not sure what that means 😄

Do you know of any cheap devices which would require Restart/Repeat we could get for testing? You have mentioned KT0803L, do you perhaps know any other?

Perhaps @JeremyKuhne knows or has some devices like that

@krwq krwq added this to the 3.0 milestone Feb 22, 2019

@krwq

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Marking as 3.0 since this is potentially blocking adding new devices

@joperezr joperezr added enhancement and removed enhancement labels Apr 16, 2019

@ZhangGaoxing

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

I think we should try to solve this problem as soon as possible. I found that the SMBus protocol devices I tried to bind were not working properly (I can't guarantee all the devices), like MLX90614 (this sensor adafruit is on sale). I don't know if the latest version of the Raspberry Pi driver supports I2C Repeated Start, I refered this post to back the I2C driver, then I realized that our lib didn't provide WriteRead() method...

I remember that a long time ago version of System.Devices.Gpio provided this method.🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.