Please review changes in github

Goebish, hexfet, mwm, FDR - in the absence of PhracturedBlue we need to keep moving. One of the activities - review and approve/merge changes in github pull requests. Even if you can't test you can review the code. We are few, so to keep 2 approvals rule we need more active participation in reviewing.

Thanks,

Victor.

It's nice of you to look. I'm not familiar with large parts of the codebase myself, I try to expand the scope though.

I merged the frskyx_drop fix as I now have means to test. The bar should probably be lower for protocols since their affect is isolated. PB has said that improvements should be merged, even if not the final solution.

Approved the yd717 change, though I understand mwm's concern.

could one of you code guru's explain what's needed to get the pending switch fixes for the 7e-256k commited, and give a hint how to do so please. I'm no great coder but as the code is working perfectly I presume it's just some minor cleanup and I should hopefully manage that

The code is very repetitive for no reason. In src/target/devo7e-256/channels.c:CHAN_ReadRawInput all the constructs like
if ((~Transmitter.ignore_src & SWITCH_3x1) == SWITCH_3x1) {
switch(channel) {
case INP_SWA0: value = (global_extra_switches & (1 << (SW_01 - 1))); break;
case INP_SWA1: value = (!(global_extra_switches & (1 << (SW_01 - 1))) && !(global_extra_switches & (1 << (SW_02 - 1)))); break;
case INP_SWA2: value = (global_extra_switches & (1 << (SW_02 - 1))); break;
}
} else ...
should be replaced with a loop over values of SWITCH_*, like
for (int i = 0; i < sizeof(SwitchTable)/sizeof(*SwitchTable); ++i) {
Switch_struct &ss = SwitchTable;
if ((~Transmitter.ignore_src & ss.inputs) == ss.inputs) {

}
}

It's not that easy - switched are grouped by alternatives - there can be 3 throw switch (actually, double throw with middle position) and double throw configured at one input set - thus some pieces of the code are if...else...
I'm not an expert of how the switch input and decoding works, but I see that I can't easily figure it out from the code and the code is redundant - that is why I asked for a clearer re-write.

It seems silpstream is not interested in rewriting the code as the pull request is pending for almost 4 months. The pull request is merely a bug fix and not a feature enhancement. IMO, leaving a buggy version in the nightly is not a good idea. Furthermore, the repetitive code was already present in the previous buggy version, I really don't understand why the buggy version shouldn't be replaced with a working one. Unless some developer comes in and picks up the code, but I don't see it happening anytime soon.

thanks for the info victzh but that's clearly way above my pay grade, i'll just keep merging the code as is into my builds as it works perfectly well where as the existing nightly code doesn't

petsmith it's not that he wasn't interested, if you read the commit logs you'll see that the changes that were asked for were above his understanding (which going by victzh's post isn't surprising as even he said it isn't easy, and he can code very well) so not knowing how to proceed only left him one option. i'm kinda sad to see him step away from deviation also as he added a lot of cool stuff in the short time he was here including code, howto posts, writing wiki pages etc etc

Yup the switch mod is working stable and very good on my u7e, I have 8 switches and 2 extra analog pots installed
on my 7e for several month now, no issues. It is really good.

OK, I merged the code to facilitate maintenance and as it is a marginal (but apparently important) platform and code is localized to this platform.

many thanks for merging it as is victzh, hopefully someone comes along and can have a look at changing it to be more suitable in the future

as you say this only impacts the u7e code and as it stands the u7e firmware isn't built in the nightlies, so it doesn't affect any other deviation users at all, but this will at least allow people with little knowledge of how git works to build for the u7e straight from the main repo using the docker containers

victzh, thanks for merging the pull request, that will make our lives easier.