fix(api): Use the get_histogram retry mechanism instead of the AsyncResponseSerial one.

[vegano1]

Overview

The GET_TOF_MEASUREMENT = "M226" GCODE for the Flex Stacker has a built-in frame caching mechanism, so the host can request the current frame to be resent in case of a timeout, invalid packet, etc. By default, the M226 GCODE sends the next frame, unless we add the R element when requesting the frame. Which means that if internally this packet is retried, then it will request the NEXT frame when we are expecting the previous. Since we have this built-in mechanism, let's disable retries when sending the "M226" GCODE in the _get_tof_histogram_frame method and instead rely on the retry mechanism in the get_tof_histogram method.

Closes: RABR-834

Test Plan and Hands on Testing

• Make sure we can send and receive serial data
• Make sure we don't resend "M226" GCODES when there is a timeout or parsing error
• Make sure we send the "M226" GCODE with the R element in case of timeout or parsing error.

Changelog

• Change the AsyncSerialConnection.send_data kwarg retries from int to Optional[int] = None
• Use the ternary operator to check that retries is not None instead of the or operator, so that 0 is considered a valid value.
• Set the retries=0 in the _get_tof_histogram_frame function when sending the "M226" GCODE

Review requests

Risk assessment

[sfoster1]

It looks good but I think we can just not commit the serial connection changes if they don't alter behavior at all, makes the pr smaller. If they do change behavior, let's add tests before merging.

[rclarke0]

Why does retries needs to be optional if we are always setting it to 0?

[vegano1]

• Because we want to have a distinction between None, 0, and >0 values.
• We also dont set the value to 0, when we use retries or self._number_of_retries, When retries is "0" which is a falsy value, it will set the retries to self._number_of_retries which in the case of the stacker is 1.

[caila-marashaj]

nice!

[vegano1]

It looks good but I think we can just not commit the serial connection changes if they don't alter behavior at all, makes the pr smaller. If they do change behavior, let's add tests before merging.

Yeah, we need the serial connection changes so that "0" is a valid value. As it stands today, "0" is a falsy value, and when using "retries or self._number_of_retries" will always be false and set the value to self._number_of_retries, which is 1 for the flex stacker.