Repository navigation
Add I2C support for NXT - #513
Merged
Merged
Conversation
nx_i2c_get_txn_status() read txns[current_txn], which is one past the end of the array once all sub transactions are done, so the same successful transfer reported success on some ports and failure on others. Check all sub transactions instead, failing if any did. A NAKed address skips the remaining ones, so checking only the last would report success. Found with a logic analyzer: a correctly acknowledged transfer on port 3 raised EIO, and NAKed reads on port 4 silently returned zeros.
If SDA is low before a START, the SoftMAC driver pulses SCL until it is released, with no limit. On a port with no sensor and the internal pull-ups off, SDA stays low (sensors provide the pull-ups), so the transfer never ended. Give up after nine pulses (enough for any slave to finish a byte, as in I2C bus recovery) and fail the transaction.
With the internal pull-ups off, as in the LEGO firmware, an open drain SCL is pulled high by the sensor alone, which doesn't reach a valid high on port 4 (shared with the RS485 transceiver). Drive SCL push-pull, as the LEGO firmware does. These sensors don't stretch the clock, so nothing else drives it. SDA stays open drain.
The sensor ports have no I2C peripheral (the TWI is wired to the AVR), so the nxos SoftMAC driver bit bangs the bus. This translates its asynchronous transaction API to pbdrv, enabling the NXT I2C sensors. Quirks handled here: - Pin 6 is also an ADC input, and SDA can't be pulled low while its channel is enabled. Disable the channel during each transfer. - Turn off the internal pull-ups, as in LEGO's driver (sensors provide them). With them on, the ultrasonic doesn't acknowledge its address on port 4. Ports 1-3 work either way. - The ultrasonic needs about 5 ms of bus idle before a START (3 ms fails, 5 ms works). Enforce 10 ms, which the nxos i2c_memory.c helpers got implicitly by polling. Without this, back to back transfers (e.g. the UltrasonicSensor identity checks) failed on every port. - Copy the write data, since the caller only provides it on the first call. - Zero the receive buffer, since the SoftMAC driver ORs received bits in. - Re-register the port for every transfer, or the next one returns the previous transaction's data without touching the bus. An absent device reports EIO within about 20 ms, an unresponsive one ETIMEDOUT. Tested with the ultrasonic sensor on each port: 3/3 constructions and 15/15 readings, with EIO in about 25 ms on the empty ports. The temperature sensor and energy meter use the same path, so they should work too. Verified against logic analyzer captures, including one of the LEGO firmware on port 4. Co-authored-by: Laurens Valk <[email protected]>
Member
|
Thanks for submitting this! We've been reworking the ports extensively so I2C devices are now automatically detected. I took the liberty to make some changes to your PR so it builds with the current driver API, including some new I2C hooks. There will be some more changes and generalizations to follow, but I'd like to merge yours first since you have tested this extensively. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes pybricks/support#2432.
This adds an I2C driver for the NXT on top of the nxos SoftMAC bit banger, so NXT I2C sensors work through UltrasonicSensor, I2CDevice, etc. Along the way it fixes a few issues in the nxos driver; see the individual commits.
Port 4 needed two changes to work: the internal pull-ups have to be off (as in LEGO's firmware), and SCL is now driven push-pull. Port 4 shares its lines with the RS485 transceiver, which I think is what was behind the port 1/4 trouble mentioned in the issue. The ultrasonic sensor also needs ~5 ms of bus idle before a START, which is now enforced. A missing sensor now fails with EIO in about 20 ms instead of timing out.
I tested with the ultrasonic sensor on each of the four ports (3/3 constructions, 15/15 readings each), and checked the bus with a logic analyzer, including a capture of LEGO's firmware reading the same sensor on port 4. I haven't tested the temperature sensor or energy meter, but they use the same path.
Best tested together with #512: without those USB fixes, reading a sensor can freeze the hub within a few minutes.