MobiFlight / MobiFlight-FirmwareSource

Firmware source for MobiFlight
MIT License
30 stars 65 forks source link

Remove usage of button and rotaryencoder libs #2

Closed neilenns closed 3 years ago

neilenns commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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.