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 missing log steps at 0.1s period #1967

Merged
merged 1 commit into from
Jun 27, 2022
Merged

Conversation

mha1
Copy link
Contributor

@mha1 mha1 commented May 15, 2022

Fixes #1966

Summary of changes

  • simulator: allow for one get_tmr10ms() tick (10ms) deviation
  • radio: switched to FreeRTOS software timer at user requested period
  • code cleanup:
    log.cpp: made variable lastLogTime static
    log.cpp: deleted unused variable g_logError
    functions.cpp, logs.cpp, sdcard.h: renamed variable logDelay to logDelay100ms to show it is scaled to 100ms increments

Thanks to @rotorman!

fixed issued where log steps would be missed due to 10ms timer resolution
- simulator: allow for one get_tmr10ms() tick (10ms) deviation
- radio: switched to FreeRTOS software timer at user requested period

code cleanup
log.cpp: made variable lastLogTime static
log.cpp: deleted unused variable g_logError
functions.cpp, logs.cpp, sdcard.h: renamed variable logDelay to logDelay100ms to show it is scaled to 100ms increments
@mha1
Copy link
Contributor Author

mha1 commented May 17, 2022

Here's a little before/after comparison. Setup is a TX16s with a non-trivial model selected and logging enabled at 0.1s for 15 minutes. The radio is not touched, just sitting there logging.

Before: current 2.7.1 implementation showing a log with 8833 log steps (roughly 15 minutes). It can be seen that the expected dt of 100ms is not well maintained with more than 12% off the requested 100ms.

 dt     cnt    freq

1 100 7735 0.876
2 110 545 0.062
3 140 545 0.062
4 150 5 0.001
5 80 1 0
6 120 1 0
7 230 1 0

After: current main with using software timers as proposed in the PR in the same setup as above with 9071 log steps at 0.1s. dt's are locked to 100ms. The single 80ms dt is the first log step entry.

 dt      cnt     freq

1 100 9070 1
2 80 1 0

I tried different scenarios but would appreciate community testing

@pfeerick pfeerick requested a review from raphaelcoeffic May 30, 2022 23:47
@pfeerick pfeerick changed the title Fixes issue #1966 Missing log steps at 0.1s period Jun 7, 2022
@pfeerick pfeerick changed the title Missing log steps at 0.1s period Fix fissing log steps at 0.1s period Jun 7, 2022
@pfeerick pfeerick changed the title Fix fissing log steps at 0.1s period Fix missing log steps at 0.1s period Jun 7, 2022
@pfeerick pfeerick added the bug/regression ↩️ A new version of EdgeTX broke something label Jun 7, 2022
@pfeerick pfeerick self-assigned this Jun 8, 2022
@mha1
Copy link
Contributor Author

mha1 commented Jun 27, 2022

@raphaelcoeffic - can please review this pr, thx

Copy link
Member

@raphaelcoeffic raphaelcoeffic left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thx!

@raphaelcoeffic raphaelcoeffic merged commit ce8cc0b into EdgeTX:main Jun 27, 2022
@mha1
Copy link
Contributor Author

mha1 commented Jun 27, 2022

@raphaelcoeffic Thx and sorry for bugging you

@raphaelcoeffic
Copy link
Member

@raphaelcoeffic Thx and sorry for bugging you

No problem! That was the right thing to do! I have quite some review work to catch up. The LVGL stuff has literally sucked up my time…

@mha1 mha1 deleted the PR-logging branch June 29, 2022 11:53
pfeerick added a commit to XING-IF/edgetx that referenced this pull request Aug 5, 2022
Fix bug introduced in EdgeTX#1967 which only affects !defined(RTCLOCK) boards
pfeerick added a commit to XING-IF/edgetx that referenced this pull request Aug 16, 2022
Fix bug introduced in EdgeTX#1967 which only affects !defined(RTCLOCK) boards
pfeerick added a commit that referenced this pull request Aug 16, 2022
Fix bug introduced in #1967 which only affects !defined(RTCLOCK) boards
@pfeerick pfeerick added this to the 2.8 milestone Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/regression ↩️ A new version of EdgeTX broke something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logging: missing log steps at 0.1s period
3 participants