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

feat: Convert PPM_UNIT build option to runtime radio setting #3993

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Sep 3, 2023

Replace the PPM_UNIT build option with runtime code and a setting in the radio data for option selection.

B&W radios, color radios and companion updated.

Screenshot 2023-09-03 at 12 38 30 pm

Screenshot 2023-09-03 at 12 39 24 pm

Screenshot 2023-09-03 at 12 38 46 pm

@raphaelcoeffic
Copy link
Member

@philmoz Woah! That was fast! I have only one remark: we should probably have 0.0 be the default format. I believe this is the default compilation option.

@philmoz
Copy link
Collaborator Author

philmoz commented Sep 3, 2023

I have only one remark: we should probably have 0.0 be the default format. I believe this is the default compilation option.

If no PPM_UNIT is specified in the build options it defaults to PPM_UNIT_PERCENT_PREC0 (no decimal).

@raphaelcoeffic
Copy link
Member

If no PPM_UNIT is specified in the build options it defaults to PPM_UNIT_PERCENT_PREC0 (no decimal).

Ok, my bad :-D

@3djc
Copy link
Collaborator

3djc commented Sep 3, 2023

I have only one remark: we should probably have 0.0 be the default format. I believe this is the default compilation option.

If no PPM_UNIT is specified in the build options it defaults to PPM_UNIT_PERCENT_PREC0 (no decimal).

Indeed, it was changed fairly recently, as PREC1 really doesn't give any benefit

@raphaelcoeffic raphaelcoeffic linked an issue Sep 5, 2023 that may be closed by this pull request
1 task
@pfeerick pfeerick self-requested a review September 24, 2023 05:59
@pfeerick pfeerick added this to the 2.10 milestone Oct 3, 2023
@pfeerick pfeerick added the enhancement ✨ New feature or request label Oct 10, 2023
@pfeerick
Copy link
Member

LGTM, and I love how this seems to have ended up with slightly smaller firmware for both colorlcd and b&w then main (after rebasing and self-compiling)!! Also shows up where the colorlcd UI has very support for the PPM_UNIT option.

Tested on X9D+2019 w/ Companion sync, as well as TX16S, 128x64 only simulator tested.

@pfeerick pfeerick changed the title chore(radio,cpn): Convert PPM_UNIT build option to runtime radio setting. feat: Convert PPM_UNIT build option to runtime radio setting Oct 10, 2023
@pfeerick pfeerick merged commit 837677b into EdgeTX:main Oct 10, 2023
40 checks passed
@sande005
Copy link

WhooHoo! Hadn't followed up on my #3163, waiting till flying season was over to see if anything has happened - either fix the build process, or add an enhancement. Very happy to see this coming in 2.10! Thank you!

ulfhedlund added a commit to ulfhedlund/edgetx that referenced this pull request Oct 12, 2023
ulfhedlund added a commit to ulfhedlund/edgetx that referenced this pull request Oct 12, 2023
@philmoz philmoz deleted the ppm-unit-setting branch January 9, 2024 10:23
@pfeerick pfeerick mentioned this pull request Mar 11, 2024
13 tasks
@sande005
Copy link

sande005 commented May 7, 2024

Unfortunately, not what was asked for in #3163 - was expecting to see ppmus values in max/min under outputs, as can be done with selecting the build option for OpenTX. Per #3163, prior custom build for ppmus did not work. Have not tested custom build to see if that issue has been resolved in ver 2.10 rc4.

@pfeerick
Copy link
Member

pfeerick commented May 7, 2024

Well, only half of what was asked (although probably the more important half)... it is now no longer a build time option, but a runtime option, but I don't think controls have been converted over fully on colorlcd. B&W radios should honor the setting better. This will need a new issue now (done: #4977), as the old one is no longer correct since it is not a build time option any more. I don't think -DPPM_UNIT_US was every valid anyway (yet?) for colorlcd - which is one of the reasons you can only pick PREC0 and PREC1 (0.0, 0.00).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ppmus build does not display ppmus in Outputs
5 participants