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: Reading optimization ( add custom timing for each mode ) #8

Merged

Conversation

RaulTrombin
Copy link
Member

Previous readings were reaching 90.271 ms,
with this PR, they have been reduced to 5 ms, which is what is expected from the 200 Hz mode on the sensor.

Criterion Plot:
iteration_times

Critetion Output:

blueos@raspberrypi-armv7:~/github/AK09915-rs $ cargo bench --bench bench -- --measurement-time 10
   Compiling ak09915_rs v0.1.0 (/home/blueos/github/AK09915-rs)
    Finished bench [optimized] target(s) in 15.06s
     Running benches/bench.rs (target/release/deps/bench-960d91f9648acce3)
Gnuplot not found, using plotters backend
new                     time:   [18.560 µs 18.906 µs 19.276 µs]
                        change: [-2.1175% +1.2418% +5.2751%] (p = 0.52 > 0.05)
                        No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe

read_register           time:   [436.26 µs 438.36 µs 440.34 µs]
                        change: [-2.0691% -0.9763% -0.1142%] (p = 0.04 < 0.05)
                        Change within noise threshold.

read                    time:   [4.9723 ms 4.9799 ms 4.9895 ms]
                        change: [-94.721% -94.483% -94.199%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) low severe
  3 (3.00%) low mild
  1 (1.00%) high mild
  3 (3.00%) high severe

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@RaulTrombin RaulTrombin marked this pull request as ready for review October 18, 2023 15:39
src/lib.rs Outdated Show resolved Hide resolved
@RaulTrombin
Copy link
Member Author

@patrickelectric can we bump release 1.2 after this?

@RaulTrombin RaulTrombin marked this pull request as draft October 19, 2023 17:48
@RaulTrombin RaulTrombin changed the title Fix: Reading optimization [WIP]Fix: Reading optimization Oct 19, 2023
@RaulTrombin RaulTrombin changed the title [WIP]Fix: Reading optimization Fix: Reading optimization Oct 19, 2023
@RaulTrombin RaulTrombin changed the title Fix: Reading optimization Fix: Reading optimization ( add custom timing for each mode ) Oct 19, 2023
@RaulTrombin RaulTrombin marked this pull request as ready for review October 19, 2023 20:15
@RaulTrombin
Copy link
Member Author

@patrickelectric @joaoantoniocardoso
Check if you agree, we have a thread sleep with (Period/2) for data ready checking,
and it will run 3 times, just in case.

Each register readings is something like 450 us.

So, with benchmark running we got this:

`blueos@raspberrypi-armv7:~/github/AK09915-rs $ cargo bench
   Compiling ak09915_rs v0.1.1 (/home/blueos/github/AK09915-rs)
    Finished bench [optimized] target(s) in 18.64s
     Running unittests src/lib.rs (target/release/deps/ak09915_rs-6c6fe00ce8f5d078)

running 1 test
test tests::set_mode_and_read_sensor ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running benches/bench.rs (target/release/deps/bench-337b80abaf75691f)
Gnuplot not found, using plotters backend
new                     time:   [18.651 µs 18.936 µs 19.225 µs]
                        change: [-1.0763% +1.3997% +4.0922%] (p = 0.28 > 0.05)
                        No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

read_register           time:   [439.97 µs 441.75 µs 443.36 µs]
                        change: [-0.2126% +0.5128% +1.6024%] (p = 0.32 > 0.05)
                        No change in performance detected.
Found 20 outliers among 100 measurements (20.00%)
  11 (11.00%) low severe
  1 (1.00%) low mild
  4 (4.00%) high mild
  4 (4.00%) high severe

read                    time:   [4.9496 ms 4.9748 ms 5.0001 ms]
                        change: [-94.838% -94.490% -94.033%] (p = 0.00 < 0.05)
                        Performance has improved.

Benchmarking Cont1Hz: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 78.8s, or reduce sample count to 10.
Cont1Hz                 time:   [987.90 ms 997.92 ms 1.0029 s]
                        change: [-1.4450% -0.4457% +0.0548%] (p = 0.63 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) low severe
  1 (1.00%) low mild
  2 (2.00%) high severe

Benchmarking Cont10Hz: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.8s, or reduce sample count to 50.
Cont10Hz                time:   [97.340 ms 99.869 ms 101.89 ms]
                        change: [-3.0231% +0.5156% +4.1546%] (p = 0.78 > 0.05)
                        No change in performance detected.
Found 19 outliers among 100 measurements (19.00%)
  12 (12.00%) low severe
  1 (1.00%) low mild
  1 (1.00%) high mild
  5 (5.00%) high severe

Cont50Hz                time:   [19.671 ms 19.917 ms 20.182 ms]
                        change: [-1.7725% -0.0235% +1.7436%] (p = 0.98 > 0.05)
                        No change in performance detected.
Found 29 outliers among 100 measurements (29.00%)
  10 (10.00%) low severe
  1 (1.00%) low mild
  1 (1.00%) high mild
  17 (17.00%) high severe

Cont100Hz               time:   [9.8792 ms 9.9473 ms 10.010 ms]
                        change: [-0.8775% +0.0229% +0.9121%] (p = 0.96 > 0.05)
                        No change in performance detected.
Found 17 outliers among 100 measurements (17.00%)
  16 (16.00%) low severe
  1 (1.00%) high mild

Cont200Hz               time:   [4.9585 ms 4.9885 ms 5.0204 ms]
                        change: [-0.7332% +0.0541% +0.8726%] (p = 0.90 > 0.05)
                        No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

I'm a bit inclined that from is ti the best way to implement it, but as a trair. The information does not imply directly on a sleep time.

@RaulTrombin
Copy link
Member Author

RaulTrombin commented Oct 20, 2023

I'm a bit inclined that from is ti the best way to implement it, but as a trair. The information does not imply directly on a sleep time.

actually It seems reasonable, but do not seems so verbose.

I was thinking into something like:

///but it's not possible
pub Enum mode {
SomeMode (mode_values{value:10, ready_check:...})
}

pub struct mode_values {
value:u8,
ready_check:std::time::Duration
}
pub type mode_var = (u8, std::time::Duration);

;

@joaoantoniocardoso

@patrickelectric
Copy link
Member

I'm a bit inclined that from is ti the best way to implement it, but as a trair. The information does not imply directly on a sleep time.

actually It seems reasonable, but do not seems so verbose.

I was thinking into something like:

///but it's not possible pub Enum mode { SomeMode (mode_values{value:10, ready_check:...}) }

pub struct mode_values { value:u8, ready_check:std::time::Duration } pub type mode_var = (u8, std::time::Duration);

;

@joaoantoniocardoso

You could just implement a trait

@RaulTrombin
Copy link
Member Author

I'm a bit inclined that from is ti the best way to implement it, but as a trair. The information does not imply directly on a sleep time.

actually It seems reasonable, but do not seems so verbose.
I was thinking into something like:
///but it's not possible pub Enum mode { SomeMode (mode_values{value:10, ready_check:...}) }
pub struct mode_values { value:u8, ready_check:std::time::Duration } pub type mode_var = (u8, std::time::Duration);
;
@joaoantoniocardoso

You could just implement a trait

@patrickelectric

@RaulTrombin RaulTrombin mentioned this pull request Oct 23, 2023
4 tasks
@RaulTrombin RaulTrombin force-pushed the reading_optmization branch 2 times, most recently from 3dc1032 to ab66ceb Compare November 1, 2023 13:20
@RaulTrombin
Copy link
Member Author

@patrickelectric

@patrickelectric patrickelectric merged commit 171cd4e into bluerobotics:master Nov 1, 2023
3 checks passed
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.

2 participants