Add block integrity check to readAxivity

wadpac / GGIRread

Functions for reading accelerometer data files

https://CRAN.R-project.org/package=GGIRread

Apache License 2.0

5 stars 3 forks source link

Add block integrity check to readAxivity #42

Closed vincentvanhees closed 11 months ago

vincentvanhees commented 11 months ago

Fixes #41

In this PR:

I have added a check of the checksum.
I have added a check of the observed versus expected sample frequency as we know that sampling frequency is not constant in Axivity. For this we introduce a new argument frequency_tol which is the tolerance in frequency variation defined as a fraction (0.1 means that we allow for a 10% difference relative to the expected frequency).
Code now automatically logs when checks are not passed.
Blocks that do not pass the checks are not resampled but imputed by a constant to reflect a non-moving accelerometer. The user can use the log to trace back which blocks were imputed and revise this step if they want.
The above additions obviously come with a computing cost, my estimate is that it slows down the code by about 15-20%, but I think for Axivity this is essential as we may not rely on the values blindly. Other cwa reading functions also attempt to do this check by default.

codecov-commenter commented 11 months ago

Codecov Report

Merging #42 (68f0249) into main (f80fc0d) will decrease coverage by 1.36%. The diff coverage is 68.86%.

:exclamation: Current head 68f0249 differs from pull request most recent head 52857dd. Consider uploading reports for the commit 52857dd to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   85.62%   84.27%   -1.36%     
==========================================
  Files           9        9              
  Lines         925      960      +35     
==========================================
+ Hits          792      809      +17     
- Misses        133      151      +18

Files Changed	Coverage Δ
R/readAxivity.R	`81.98% <68.86%> (-3.93%)`	:arrow_down:

l-k- commented 11 months ago

I was looking at the firmware code, and documentation here suggests that old files with a 0 at offset 24 don't have checksums. We are already treating such files differently here, so maybe we should do the same for checksums.

I will spend more time on this review tomorrow (Wednesday).

vincentvanhees commented 11 months ago

thanks @l-k- for all your input. I will merge this and make a new GitHub release now. CRAN is having a summer break, so next CRAN release will have to wait a bit.