Adds support also for correct Alps EC10E roller hookup

[rotorman]

Supports, in addition to factory T16, TX16S mistaken roller hookup, also correctly wired up Alps EC10E encoder on T16 Family radios.
Thx to Mike Blandford for the code (https://www.rcgroups.com/forums/showpost.php?p=46929617&postcount=9974)

Corrected roller schematic and pictures in EdgeTX PR 261: EdgeTX/edgetx#261

[olliw42]

@rotorman
hey, many thx for this! much appreciated
may I ask: does this PR has any effect for the normal hardware, or is it really totally indistinguishable for a user from the old code?
I may not have the time to test it out fully, so some experience of you concerning this would be helpful
I wonder if one should wrap it with a define for as long as it's not totally clear what the effect for normal hardware would be

[olliw42]

the encoder check code change is in the branch for

#if (defined(RADIO_FAMILY_T16) && !defined(RADIO_T18)) || defined(RADIO_TX12)

but the change of the granularity is only for

#elif defined(RADIO_FAMILY_T16) && !defined(RADIO_T18)

while it is not changed for

#elif defined(RADIO_TX12)

Is this correct? I do not know, but it looks like this might not be correct

[rotorman]

Hi Olli, yes I tested the change with the factory RM TX16S roller board AND with my modded board - works with both. Mike also said this that this code will work with both, and it sure does.

About the GRANULARITY - this could be simplified to

#if defined(PCBSKY9X)
  #define ROTARY_ENCODER_GRANULARITY (2 << g_eeGeneral.rotarySteps)
#elif defined(RADIO_TX12)
  #define ROTARY_ENCODER_GRANULARITY (1)
#else
  #define ROTARY_ENCODER_GRANULARITY (2)
#endif

But I wanted to left the code for now so, that it is easy to follow what I changed (just one value). But indeed, might make more sense to change it to be the short form. I'll fix this ASAP.

About TX12, dunno if this works.

[rotorman]

I gave it some further thought, and yes, this should be 2 also for TX12, as the same code is used in rotary_encoder_driver.cpp for TX12 as well. As Mike's code works with both encoder hookups, 2 is the correct value. I'll do a new commit ASAP.

[olliw42]

works

I didn't meant to imply it would not work, I have seen it myself that it works, but "works" is not equal to "works", this word can mean all sorts of things

e.g. it is also said that the original roller code "works", yet every once in a while it could give a wrong reading making the selection jump unexpectedly because it is a workaround for that hardware flaw (that's at least what I was told by a dev that this is what it is), so, it does work, but also does not work in the sense of how it would work if there wouldn't be the hardware issue,... so "works" is not equal to "works"

what I'm trying to understand better for the native hardware is how it "works" in comparison to the original code, is it doing really exactly the same? Is it maybe even better? Is there any difference as compared to before?

[olliw42]

@rotorman
ok, added it to the dev branch
I did not merge your PR since I wanted to keep the original code, just in case ... and since this is a PR to the base branch which I've made to not be the dev branch ... I really need to sort this out at some point ...

MANY THX for this

[rotorman]

Thx. And my bad in the first place, making a pull request to the wrong branch.

[olliw42]

well, it's not your fault I think, since it's the default to go to the base branch, as said, I need to sort this out
the reason I modify the base branch is because I want users to be able to get quickly to the latest release, and especially the .bins, not sure if it is possible to show a specific branch as default and set the base branch to something else ... maybe I just have to accept how github is ;(

[rotorman]

@3djc made a PR for OpenTX 2.3 of Mike Blandford code for roller 3h ago as well ;)
opentx#8564