xross
commented
1 week ago Thanks for the detailed explanation!
Closed shuchitak closed 1 week ago
xross
commented
1 week ago Thanks for the detailed explanation!
Changes in this PR -
Have NACKED state follow the same path as ACKED. In the current checker, on receiving or sending a NACK, the checker transitions to NACKED_SELECT -> STOPPING_0 -> STOPPING_1 states where it expects a stop or stop/start from the master. I removed these 'artificial' stop states and instead changed NACKED to follow the same path as ACKED, where stop/start/repeated_start conditions are detected through changes in SCL and SDA (in the CHECK_START_STOP state). This makes the code simpler, also more intuitive since the state transitions now follow changes in SCL and SDA.
Related to above, I also noticed that when stopping through the NACKED_SELECT -> STOPPING_0 -> STOPPING_1 states, repeated start wasn't being detected. After having NACKED follow the same path as ACKED, repeated start is being detected fine, as part of the CHECK_START_STOP state. As a result, all the nostop.expect files now have 'Repeated start bit received' instead of 'Start bit received' which is correct since in the 'no_stop' tests the master sends a start without sending a stop bit (repeated start), so the checker should detect and log a repeated start.
I also noticed that the setup/hold time check functions (check_hold_start_time, check_setup_start_time etc.) execute the check only for 'speed == 100 or speed == 400'. However, in cases where the slave is clock stretching, the speed detected in the checker is not the same as the speed the master is running at. As a result, the checks don't run at all, leading us to not detect the start bit setup timing violation during repeated start when slave is clock stretching case (test case test_master_clock_stretch.py), as pointed by @QuinnWang in https://github.com/xmos/lib_i2c/pull/83. To fix this, I've added an additional parameter called original_speed which is the speed at which the I2C master is running at (and not the speed detected in the checker). This parameter can be optionally set in the test, otherwise, it's same as the expected speed. I've changed the timing checks to run using the original_speed parameter. test_master_clock_stretch.py is not currently setting the original_speed. However, if it does, the start bit setup timing would get violated in the test. I'll make a PR to fix that next.