Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup retry logic in I2C drivers, implement better retries in main event polling loop, remove delays, possibly return NaNs #588

Open
tyeth opened this issue May 23, 2024 · 1 comment

Comments

@tyeth
Copy link
Contributor

tyeth commented May 23, 2024

A few of the sensors are more unreliable than most.
They fail before a data read and return false.
The firmware then loops over the event polling code again and detects the sensor is still overdue.
This repeats.
If they failed forever, then the code is basically in a tight loop polling the sensor indefinitely. The serial monitor can become flooded (along with the UI if available, but so far the main issue is around whether to retry inside the driver).

A few of the I2C drivers retry the data read inside the driver code to ensure the user's best chance of recording their desired data.

This deviates from the idea of being a short rapid interrupt-like method to fetch the data by using delays between read attempts.

In an ideal world they would quickly fail, and the main event loop would retry them a sensible number of times before marking the event as passed and updating the next poll time.

It's possible we may want to send a NaN in this situation (although the current thinking is NaN means something specific - i.e. that a target is not currently in the area for a proximity sensor), or bubble up another kind of error / msg to the IO website and accept the sensor data read event returned unsuccessfully.

Again there is a fair bit of code repetition for retries, and the main thing that came up was trying to monitor the millis while checking the result of a conditional, a possible rollover situation, which leads to a horrific looking for loop with the main execution block indistinguishable from the conditions (VL53L4CX development #564 ).
It was an interesting excercise to explore the idea of a RETRY_UNTIL_TIMEOUT macro, but unnecessary if we fix the main event loop to retry more efficiently. For posterity: https://chatgpt.com/share/b760dd1d-5bdf-4f5d-b3ae-ec762eab6207

@tyeth
Copy link
Contributor Author

tyeth commented Jun 14, 2024

So I went a tiny bit further, and made a macro to retry a function with a conditional check and a timeout.
It's here in WipperSnapper.h:
main...pb-error-msgs#diff-ea75f03b01eafd5a02c644c85f7ddc45401eda065b1d002832b12e85e5af73dbR75-R108

/**************************************************************************/
/*!
    @brief  Retry a function until a condition is met or a timeout is reached.
    @param  func
            The function to retry.
    @param  result_type
            The type of the result of the function.
    @param  result_var
            The variable to store the last result of the function.
    @param  condition
            The condition to check the result against.
    @param  timeout
            The maximum time to retry the function.
    @param  interval
            The time to wait between retries.
    @param  ...
            The arguments to pass to the function.
*/
/**************************************************************************/
#define RETRY_FUNCTION_UNTIL_TIMEOUT(func, result_type, result_var, condition, \
                                     timeout, interval, ...)                   \
  {                                                                            \
    unsigned long startTime = millis();                                        \
    while (millis() - startTime < timeout) {                                   \
      result_type result_var = func(__VA_ARGS__);                              \
      if (condition(result_var)) {                                             \
        break;                                                                 \
      }                                                                        \
      if (startTime < millis()) {                                              \
        startTime = millis(); /* if rollover */                                \
      }                                                                        \
      WS_DELAY_WITH_WDT(interval);                                             \
    }                                                                          \
  } ///< Retry a function until a condition is met or a timeout is reached.

Separately the retry-loop branch is going on here:
main...retry-loop

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

No branches or pull requests

1 participant