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

Remove usage of button and rotaryencoder libs #2

Closed
neilenns opened this issue Sep 4, 2021 · 5 comments · Fixed by #10
Closed

Remove usage of button and rotaryencoder libs #2

neilenns opened this issue Sep 4, 2021 · 5 comments · Fixed by #10
Assignees
Labels
Code cleanup Changes that clean up unused libraries, memory issues, etc. but do not change functionality.

Comments

@neilenns
Copy link
Contributor

neilenns commented Sep 4, 2021

From the MobiFlight issue elral has good changes to include:

I moved the required functions from button and rotaryencoder libs to MFEncoder.cpp. Therefore these libs are no longer required and I do not expect changes in these libs anymore (at least which would be required for MF). Interesting side effect is that this has reduced the RAM usage by ~210byte and Flash by ~2,1kB!?? If you are interested I can provide the changes and an additional test would be good if especially the FAST detection works with all encoder types.

@elral
Copy link
Collaborator

elral commented Sep 5, 2021

As far as I know how to issue pull requests I will try to make my first one ;)
I hope it takes not too much time to figure this out...

@neilenns
Copy link
Contributor Author

neilenns commented Sep 5, 2021

@elral the easiest way is if you are in VSCode to use the GitHub Pull Requests and Issues extension.

  1. Fork this repo
  2. In VSCode open the GitHub Pull Requests and Issues tab and look for issues assigned to you (I'll assign this one to you). Hover over the issue and click the little right arrow to automatically make a new branch for the work
  3. Do your changes and push them to your repo
  4. Come back to the GitHub Pull Requests and Issues tab and hit the weird icon with two circles, an arrow, and a + sign to open the pull request, fill in the form, and voila!

elral added a commit to elral/MobiFlight-FirmwareSource that referenced this issue Sep 6, 2021
@elral
Copy link
Collaborator

elral commented Sep 6, 2021

@danecreekphotography, thanks a lot for your hints. Took some time to understand, and in the end I had to do step 4 on the github page. Didn't find what you explained.
Hopefully everything went well.
Regards
Ralf

@elral
Copy link
Collaborator

elral commented Sep 8, 2021

I "found" that the original library (in platformio.ini "mathertel/RotaryEncoder @ ^1.5.2") got recently some updates. The button and TicksPerSecond lib is not used anymore. Acceleration can now be done via "::getMillisBetweenRotations()". There are some more little changes in MFEncoder.cpp required (which I prepared already). RAM usage is reduced by ~300Bytes!?

BUT encoders with 4 Steps per cycle are NOT supported anymore. Are there at all these rotary encoder (for inputs, not for detecting shaft rotating w/o detents) existing? For me I never had them and never saw them, only 1 and 1 detents per cycle. Does this somebody know?
If they exist and used, plan B has t come (or is it C?)

@neilenns
Copy link
Contributor Author

neilenns commented Sep 8, 2021

@elral Just to reduce the amount of churn happening in the firmware in a single release (and there's a lot of combined changes for this PlatformIO move), my suggestion is to stick with what you already did that's based on the current version of the library that's well understood for MobiFlight usage.

@neilenns neilenns added the Code cleanup Changes that clean up unused libraries, memory issues, etc. but do not change functionality. label Oct 14, 2021
@neilenns neilenns added this to In progress in Code cleanup Oct 17, 2021
@neilenns neilenns moved this from In progress to Review in progress in Code cleanup Oct 17, 2021
Code cleanup automation moved this from Review in progress to Done Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code cleanup Changes that clean up unused libraries, memory issues, etc. but do not change functionality.
Projects
No open projects
2 participants