Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[FL-3880] Fix cumulative error in infrared signals #3823

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

gsurkov
Copy link
Member

@gsurkov gsurkov commented Aug 5, 2024

What's new

  • Transmitted infrared signals no longer exhibit a cumulative error due to rounding.
  • Fix the wrong length of the leading carrier pulse.

Verification

  1. Compile and flash this branch
  2. Attach a logic analyser to the PA7 pin
  3. Begin the capture and run ./fbt launch APPSRC=infrared_test
  4. Look in the debug log output, note the predicted signal duration
  5. Measure the captured signal duration from the first rising edge to the last falling edge
  6. The signal duration should be within predicted limits.

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

Copy link

github-actions bot commented Aug 5, 2024

Compiled f7 firmware for commit 151d007a:

@gsurkov gsurkov marked this pull request as ready for review August 5, 2024 12:50
@CookiePLMonster
Copy link
Contributor

If you want to preserve the remainder of the division, isn't https://en.cppreference.com/w/c/numeric/math/div or div + rem a better way to go here? Might potentially also let you keep calculating on integers, without the need to cast to floats.

@gsurkov
Copy link
Member Author

gsurkov commented Aug 5, 2024

The code was already dealing in floats (cycle_duration is a float), and the remainder has to be a float because it is always less than 1. I could probably calculate it in terms of microseconds instead of carrier periods, but I can't see any benefits of doing so without profiling.

@CookiePLMonster
Copy link
Contributor

The code was already dealing in floats (cycle_duration is a float), and the remainder has to be a float because it is always less than 1. I could probably calculate it in terms of microseconds instead of carrier periods, but I can't see any benefits of doing so without profiling.

Ah, I see. In this case you can likely use modf to get the integral part and the fractional part of the division, and store the fractional part as a remainder. Should be similar to your current solution, but without rounding and the awkward subtraction used to calculate the remainder.

@gsurkov
Copy link
Member Author

gsurkov commented Aug 5, 2024

Proper rounding (up and down) is preferable in this case because it spreads extra pulses more evenly across time. Suppose we get an intended duration of 3.9 pulses, which will turn into 3 this time and 1 extra pulse next time if we use modf, and 4 and 0 with round, which will be more representative of the intended signal. Same goes for small fractional parts, too.

@skotopes skotopes merged commit 0b19fd2 into dev Aug 7, 2024
11 checks passed
@skotopes skotopes deleted the gsurkov/3880_ir_timings branch August 7, 2024 03:05
ofabel pushed a commit to ofabel/flipperzero-firmware that referenced this pull request Sep 26, 2024
* Correct for pulse duration cumulative discrepancy
* Add infrared test application
* Build infrared_test_app for f7 only

Co-authored-by: あく <alleteam@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants