Closed whoenig closed 3 years ago
@ataffanel @krichardsson It would be nice if you can comment on this design, before I work on an actual implementation of it.
Looking good! Pushing the IMU data the same was as other sensors would be nice and we could get rid of some nasty dependencies as well.
One property that is lost in this design is "prioritisation" between sensor types. Even though it is not utilised in a structured way today, it is possible to set the queue length for different sensors. If one sensor pushes lots of data (too much) into the queue today, it will be discarded while other sensor data still will be used as it is pushed to other queues. In the suggested design we might discard the "wrong" sensor data instead. One may argue that we should have a different mechanism to throttle sensor data anyway.
There was a in-person discussion about the issue of dropping data: We will add code to detect if the queue overflows and warn the user (only once!) using a DEBUG_PRINT. Then, the queue size in the firmware should be increased accordingly to find a good length. For the current implementation, we do not have such warnings, so we actually don't know if our queues are too large or too small.
The current design of the state estimators has multiple issues:
A possible solution to address these issues is:
estimator.c
add a new unified queue:estimator.c
, we can now fill definitions for all the enqueue functions to just push data in the queue. In all state estimators, we simply extract items from the queue and ignore measurements that we do not want to handle. There is no need anymore to define additional enqueue functions in each state estimator. This solves both problems 2) and 3).stabilizer.c
, we always callsensorsAcquire(&sensorData, tick);
to ensure that current logging variables are updated. In the future, we should eliminate the sensorData struct entirely, but this would be the scope of a separate issue.