Skip to content

Conversation

@laurensvalk
Copy link
Member

It turns out the ColorLightMatrix needs the NACK to get going at all, so make sure we always send one before we start waiting on the keep-alive timeout. In b17e503, we removed grace period of 6 keep-alive messages to go unanswered, which was essentially masking this issue so the device worked before.

@laurensvalk
Copy link
Member Author

Might need some testing to ensure that the extra NACK does not upset any other sensors.

@coveralls
Copy link

coveralls commented Jun 14, 2025

Coverage Status

coverage: 57.141% (+0.02%) from 57.124%
when pulling 4a4714a on fix-colorlight-matrix
into 3198a85 on master.

@laurensvalk
Copy link
Member Author

The EV3 color sensor needs this too. It was also relying on the grace period of 6 keep-alive failures and stopped working when we removed that in b17e503. The workaround for that was to set an initial mode in 91acfb3 but the initial NACK takes care of it too.

@laurensvalk
Copy link
Member Author

laurensvalk commented Jun 16, 2025

It looks like we had the allowance for 6 errors all the way back to the start ab61ffd since adapting it from ev3dev.

        // make sure we are receiving data
        if (!data->data_rec) {
            data->num_data_err++;
            DBG_ERR(data->last_err = "No data since last keepalive");
            if (data->num_data_err > 6) {
                data->status = PBIO_UARTDEV_STATUS_ERR;
            }
        }

@dlech, do you recall why it was there?

EDIT: References: ev3dev/lego-linux-drivers@90878a4

@laurensvalk
Copy link
Member Author

laurensvalk commented Jun 16, 2025

Might need some testing to ensure that the extra NACK does not upset any other sensors.

This is working fine on SPIKE Medium motors, the Technic XL motor, the Force Sensor, the Color Distance Sensor, the Tilt Sensor, Boost motor, Color Sensor and Ultrasonic Sensor. Let's proceed and keep an eye on it as we go.

@laurensvalk
Copy link
Member Author

PSA: @antonvh: We're making a slight update to the protocol by sending an extra keep-alive message right after syncing up. All LEGO sensors seem to be expecting this and it might work just fine on yours, but letting you know just in case you want to test it :shipit:

It turns out the ColorLightMatrix needs the NACK to get going at all, so make sure we always send one before we start waiting on the timeout.

In b17e503, we removed grace period of 6 keep-alive messages to go unanswered, which was essentially masking this issue so the device worked before.
This was added in 91acfb3 to get the sensor going. Since 6c5c1f0, we are doing it by sending a keep-alive message, so we don't need to set any mode on this sensor. It seems better to stick to the default reflection mode.
@laurensvalk laurensvalk force-pushed the fix-colorlight-matrix branch from 7c5fcf2 to 4a4714a Compare June 16, 2025 07:25
@laurensvalk laurensvalk merged commit 4a4714a into master Jun 16, 2025
29 checks passed
@dlech dlech deleted the fix-colorlight-matrix branch June 16, 2025 14:09
@dlech
Copy link
Member

dlech commented Jun 16, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants