fivdi / i2c-bus

I2C serial bus access with Node.js
MIT License
348 stars 57 forks source link

Add timeout to bus scan #69

Closed toddbu closed 5 years ago

toddbu commented 5 years ago

Following the instructions in this repo, I added a software i2c bus to my RPi 3 B as bus #3. If I don't connect any devices to GPIO23 and GPIO24 and try to run the scan function, it takes approximately two minutes to scan the bus and return the results. It's the exact same behavior as running i2cdetect -y 3 gives with the same hardware setup. Researching this issue my best guess is that the SCL pin is staying low because there are no devices on the bus to pull the line high so the end result looks like one long clock-stretch.

Looking at the scan function in the i2c-bus.js file at line 595, it appears that each device address is tried regardless of how long the previous receiveByte function calls take. It would be great if there was a timeout option that could be passed into the scan function that would limit the total amount of time that the function is allowed to do its work. To simplify the design, I don't think that it would be necessary to cancel any active receiveByte call to match the timeout exactly. Instead, it would probably be sufficient to check the total time spent in the scan call after each call to receiveByte and exit with a timeout error if we've been in the call too long.

As an alternative, if this is a clock-stretching issue and there's a way to detect that condition on the receiveByte call then I think that it would be useful to immediately fail the scan call with an error indicating the problem. Not only would it solve this problem but it would also allow easy detection of a bad device that is holding the SCL pin low for an excessive period of time.

fivdi commented 5 years ago

Following the instructions in this repo, I added a software i2c bus to my RPi 3 B as bus #3. If I don't connect any devices to GPIO23 and GPIO24 and try to run the scan function, it takes approximately two minutes to scan the bus and return the results. It's the exact same behavior as running i2cdetect -y 3 gives with the same hardware setup. Researching this issue my best guess is that the SCL pin is staying low because there are no devices on the bus to pull the line high so the end result looks like one long clock-stretch.

If nothing is connected to GPIO23 or GPIO24 then it isn't really an I2C bus. In order for it to be an I2C bus pullup resistors will be needed on the SDA and SCL lines. If pullup resistors are used, as required by the I2C bus specification, the scan method will run quickly, even if no I2C devices are connected to the bus.

I'm very very hesitant about adding a timeout to the scan method. I'm far more inclined to let the operating systems and drivers decide what needs to be done and allow them to wait as long as they see fit rather than override their decisions.

If you want to avoid adding the pullup resistors to the SDA and SCL lines and would like scan to run faster then there is an option. If scan is called without startAddr and endAddr arguments it will scan all addresses and take a long time if there are no pullup resistors. However, if scan is called with a single address or an small address range it will run a lot faster. Your code can then decide if the operation took too long and terminate.

fivdi commented 5 years ago

Adding the following line at the end of /boot/config.txt and rebooting the Raspberry Pi may also resolve the issue:

gpio=23,24=pu

It's a bit of a hack, but it enables the internal pullup resistors on GPIO23 (SDA) and GPIO24 (SCL) which will pull them high and should result in scan running faster.

toddbu commented 5 years ago

Thanks for the quick feedback. In general, I agree with your concerns about letting the OS and drivers do the work, and to be honest the timeout is a bit of a hack so I don't blame you if you don't want to implement the change. I also agree that a bus without anything hooked up really isn't a bus, except that it is. :-) If we list the busses using i2cdetect -l then the bus appears in the list, so our code that walks all the busses looking for devices can't tell the difference between a bus that's wired and one that isn't. For our use case, a bus left unwired is an acceptable condition. All we care about is which busses are in use and what devices are attached to them. Enabling the internal pullup resistors might do the trick if it doesn't interfere with the operation of the bus. I'll give it a try.

I did a little more investigation since initially writing this report. Before calling the scan function, I invoked a readByte(0x03, 0x00, cb) on the bus and then looked at the error on the callback. If there's no error then it means that the bus is functioning and that there's a valid device present. (I don't think that the address matters in this case.) If there's an error then what I saw in my testing on the software i2c showed different on a wire bus than an unwired bus. The wired bus returned { [Error: , Remote I/O error] errno: 121, code: '', syscall: 'readByte' } and the unwired bus returned { [Error: ENXIO, No such device or address] errno: 6, code: 'ENXIO', syscall: 'readByte' }. I was able to craft a workaround using this information.

I tried looking up ENXIO and the descriptions were kind of vague. The best that I found was this...

ENXIO
--
  Returned by I2C adapters to indicate that the address phase
  of a transfer didn't get an ACK.  While it might just mean
  an I2C device was temporarily not responding, usually it
  means there's nothing listening at that address.
  
  Returned by driver probe() methods to indicate that they
  found no device to bind to.  (ENODEV may also be used.)

Reading that description makes me think that I could still get that same code on a wired bus under certain conditions, so I'm not sure that I'm ready to recommend that you consider adopting it as a way to determine if the bus hardware is missing and the scan can be skipped. For my purposes it seems to solve the problem.

Anyway, thanks again for the feedback. I love this library as it makes working with i2c devices easy. I was super impressed that the 4.x upgrade was painless given the amount of refactoring that you did. Kudos to you on the great work!

Please feel free to close this issue if you want.

fivdi commented 5 years ago

I also agree that a bus without anything hooked up really isn't a bus, except that it is. :-) If we list the busses using i2cdetect -l then the bus appears in the list

Indeed.

The wired bus returned { [Error: , Remote I/O error] errno: 121, code: '', syscall: 'readByte' }

This is the error that will occur on relatively recent Linux kernels. On older kernels it's something like { [Error: , I/O error] errno: 5, code: '', syscall: 'readByte' }. This probably isn't something that you need to worry about though.

Anyway, thanks again for the feedback.

You're welcome.

I love this library as it makes working with i2c devices easy. I was super impressed that the 4.x upgrade was painless given the amount of refactoring that you did. Kudos to you on the great work!

Thank you very much.