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

Fix rest detection timescale #305

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

wigwagwent
Copy link
Contributor

Sensor fusion expects the time to be in seconds while rest detection expects it to be in microseconds. This makes is so when the update sensor fusion is called the rest detection now gets the time in micros. Also linear acceleration was missing parentheses giving wrong results (only lsm6dsv uses this to my knowledge)

Sensor fusion expects the time to be in seconds while rest detection expects it to be in microseconds. This makes is so when the update sensor fusion is called the rest detection now gets the time in micros
@wigwagwent wigwagwent force-pushed the SensorFusionInconsistencies branch from 9ae0021 to 8c944b6 Compare December 1, 2023 03:43
wigwagwent added a commit to wigwagwent/LSM6DSV16X that referenced this pull request Dec 1, 2023
@ButterscotchV
Copy link
Member

The time scale can probably be moved to RestDetection.h can't it? I think it's odd to have it different, otherwise this does look good I think?

src/sensors/SensorFusion.cpp Outdated Show resolved Hide resolved
@wigwagwent
Copy link
Contributor Author

I changed the rest detection to be in seconds instead of in microseconds like it was before. I am not able to test this. The best way to test would be see if you can still calibrate a bmi160.

Copy link
Contributor

@0forks 0forks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, need to change the types to sensor_real_t, otherwise it all gets rounded and deltas become 0.

Also minor inconsistency with tabs/spaces, but we need to fix that across the whole codebase honestly.

@@ -20,14 +20,14 @@
struct RestDetectionParams {
sensor_real_t biasClip;
sensor_real_t biasSigmaRest;
uint32_t restMinTimeMicros;
uint32_t restMinTime;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t -> sensor_real_t

@@ -235,7 +235,7 @@ class RestDetection {
private:
RestDetectionParams params;
bool restDetected;
uint32_t restTimeMicros;
uint32_t restTime;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t -> sensor_real_t

restDetected = false;
}
#endif
}

void updateAcc(uint32_t dtMicros, sensor_real_t acc[3]) {
void updateAcc(uint32_t dt, sensor_real_t acc[3]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t -> sensor_real_t

Eirenliel

This comment was marked as outdated.

@Eirenliel Eirenliel merged commit 993d35a into SlimeVR:main Mar 8, 2024
1 check passed
l0ud pushed a commit to l0ud/SlimeVR-Tracker-ESP-BMI270 that referenced this pull request Apr 1, 2024
Sensor fusion expects the time to be in seconds while rest detection expects it to be in microseconds. This makes is so when the update sensor fusion is called the rest detection now gets the time in micros

Co-authored-by: Eiren Rain <Eirenliel@users.noreply.github.com>
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