button input select (toggle switch recognition)

I've implemented the feature described here to learn more about the gui. Test builds are uploaded for the Devo 10 and 8. The switches, sticks, and pots can be moved to select both source and switches in the mixers. Deviation uses switch position (not just the switch itself) so to select you move the switch to the position you want. Analog inputs must change at least 25% to be recognized to reduce inadvertent changes. The test builds are based off the current tip.

I haven't implemented the other transmitters yet, and I have a few concerns about my implementation that I've commented in the commit here . Comments welcome - I may have taken the wrong approach. The current code does not change any function signatures to reduce the number of changes. It might be cleaner (but take more code) to do it another way. Would like to make any design changes needed to get a pull request merged.

I chose to add a pseudo-buttonpress on an input (analog or switch) change. Two main things had to be added. First is creating a button press event from a change in one of the inputs (analog or switch). Second was making the GUI respond appropriately to the event.

I made a single pseudo-button (BUTTON_INPUTS) handle all input changes for a couple reasons. Since deviation uses switch position instead of just switch, multiple switch positions change when you move a single switch. The use case here requires knowing the switch position "moved to" so treating each switch position as a separate button doesn't make sense. There's a max of 32 buttons, and adding each input as a separate button approaches the limit.

The downside to the single button press for all inputs is that the input value needs to be passed down to the value callback that sets the item value. The existing TextSelect callback just takes an up/down direction value. Two steps are involved to pass along the source value. The button value normally carries a bit-field indicating the active buttons, but when BUTTON_INPUTS is set in the flags the button holds the source value. The press callback is modified to check for BUTTON_INPUTS events, move the source value to dir, and immediately invoke the callback. At the bottom, INPUT_SourceSelect checks for an overloaded dir value and sets the source directly if found.

Secondly, not all TextSelect elements should respond to BUTTON_INPUTS events. I defined a TextSelectInput guiObject Type to use with TextSelect GUI objects. The GUI button handler ensures BUTTON_INPUTS events are only passed to TextSelectInput types.

It's a nice feature on the 10. Not sure about the GUI screens - at least in the emulator you can only select an item with the buttons, and moving a switch/stick/pot only works if the source or switch box is the selected item.

Comments invited!

Nice to see someone is taking it by the horns!
Im not that in to programming, even if I like the small amount of what I can do, but I will follow this thread with a smile on my face =)

In my world, there should be some kind of protocoll for stick inputs in the firmware, isnt it just that easy that you can add a few lines code in the "selection-line" to use that protocoll for inputs? Like a shortcut?
Insted of choosing ex. throttle, elevator with the buttons, you select like "ToggleSW", and that activates the code for toggle what kind of switch you want to use..

As I said, Im not that good a coding =)

PhracturedBlue wrote: Directly manipulating the TextSelect 'type' value in the page code is really nasty. Instead create a new GUI_CreateTextSelectInput() wrapper function which does what you need

Agreed. Is adding a new flag/parameter to guiTextSelect okay, or try to squeeze in a flag to one of the existing fields? The first pass I didn't change any of the structures. If adding a new parameter is okay then one thought is to make it another callback pointer to a function that knows how to handle an input value. It would only be set on GUI items (e.g. Src and Stick) where it makes sense.

PhracturedBlue wrote: In buttons.c I dont' knwo why you have this:

static s32 last_inputs[NUM_INPUTS+1];

It seems you only need to capture the analog inputs, not all inputs. you should likely use a 16 bit value (or even just an 8 bit value since you don't need precsion here). Try to minimize the RAM usage of static variables

The switch positions need to be saved as well to detect changes. Was thinking of changing those to bitfields but need to figure out how to make that flexible for the different transmitters. Good idea to save a reduced version of the analog values.

PhracturedBlue wrote: I don't much like the button approach if I understand it correctly. I think you created a virtual button 'BUTTON_INPUTS' which appears in both the button flags and as a button in capabilities, and in reality this is just a hook to detect an input switch? That generates a callback to determine which input change happened? If I understand that, it is an innovative way of solving the problem, but I don't think it is the right approach. I need to think about it further as to how to better manage it.

I think conceptually the way the textselect works is good, but I think we need a better way to detect button events. Maybe textselectinput objects need a different callback function and we handle it as an independent event rather than using the action_cb

Yes, treating an input change as a virtual button was the least intrusive (aka easiest) way I saw to notify the GUI object of the event. Also the button handler in the main loop is a convenient place to check for input changes and determine which input changed. The friction is that the existing button callback machinery doesn't have an way to pass a value to the notified object. I squeezed the selected source value into existing fields but it could be done more cleanly.

Landin wrote: , isnt it just that easy

Landin wrote: Insted of choosing ex. throttle, elevator with the buttons, you select like "ToggleSW", and that activates the code for toggle what kind of switch you want to use..

The current implementation doesn't require any "activation". If a Src or Switch box is selected and an input (switch, stick, pot) is changed, then the input that changed will become the new value of the Src or Switch. I considered putting some kind of safety to prevent inadvertent changes but it's no different than accidentally hitting the L or R button when the Src or Switch box is selected.

Sounds good. I'll head in that direction.

One possible alternative to the virtual button design would be to set up another callback mechanism just for the input changes. The code in buttons.c would move to it's own function called from the main loop to scan the inputs. A handler for input change events would be added to gui.c, which would pass the events to gui objects that could handle them. The "source select" callbacks could pass both the source and it's value which might enable other applications - for example setting scale or expo with the throttle stick.

I'd thought about changing the button callback signature to include a source variable, but it seemed that would require touching quite a bit of code.

Please take a look at this diff . Only applied to the simple_row_cb so far; wanted to get feedback before changing elsewhere.

The event detection code is moved to INPUT_CheckChanges which is called in EventLoop. Didn't want to do it in the ISR that updates the mixers. Still need to make the space reduction. For this feature the input changes need only be handled by the GUI so I skipped creating an input event and handlers. If a change is detected, GUI_HandleInput is called. And only the selected gui object can receive an input event (for this feature) so GUI_HandleInput is simple.

An InputValueCB is added to TextSelect. Perhaps the INPUT_SelectSource change could be done more cleanly but nothing better has occurred to me so far.

Okay, should be pretty close now . Implemented your suggestions. Reduced the space for the input history. The simple_row_cb is smaller in the original form so changed it back. Implemented the feature for all mixers source and switch fields on 128 and 320 pages.

I've uploaded test builds for the 10, 7e, 8, and 12.

From the function size utility on the devo10 target:
Current tip:
154116 TOTAL function size
18976 ROM data
173092 TOTAL

select_source branch:
154530 TOTAL function size
18962 ROM data
173492 TOTAL

I've extended the input select feature to the other fields where it made sense to me: Safety, Trim Input, Timer switches, Trainer Switch, and Datalog Enable. I've uploaded test builds for the 8, 10, and 12.

Unfortunately the 7e build is 256 bytes past its size limit. I'll look at reducing the code size, but don't know if I can make it fit. I'd rather have all the same type fields work the same, but another solution would be removing the feature from one or more of the list above.

Should be ready for a pull request after that. I've tested pretty well, though only emulator for the color pages. Needs testing on a 7e with switch mods (when 7e works...)

devo10 size from select_source branch:
154948 TOTAL function size
18964 ROM data
173912 TOTAL

Would it make sense for this feature to not work on the 7E?

No, please not remove USB HID!!!! I using this for simulator - PhoenixSIM and few another sim.

Not going to be able to reduce the new/changed code by 30% to fit the 7e. I reduced the largest new function by 8% - 12 bytes. Can't even remove that much from the smaller edits.

Pull requested. Wasn't confident about forcing a rebase, but currently no conflicts.

This was interesting. Not royal, but maybe knightly Feeling more sympathy for the GUI team at the day job - their framework isn't as nice.