Function ("Fn") button for hotkeys, changing hotkeys themselves via webconfig

[bsstephan]

This is inspired by my confusion and subsequent conversation on Discord after having my Analogue consoles' system reboot and system menu commands hotkeyed to START+SELECT+UP/DOWN, which of course were not getting sent as they are GP2040-CE hotkeys. This allows for a non-gamepad button, Function ("Fn"), to contribute to an "aux" state, and adds the ability to reconfigure hotkey combos, or define new combos, in the web configurator, using a combination of dpad, button, and aux masks to define the desired combination of buttons.

For example, START+SELECT+DOWN -> DP Mode is now defined as

buttons GAMEPAD_MASK_S1 | GAMEPAD_MASK_S2
dpad    GAMEPAD_MASK_DOWN
aux     0
action  HOTKEY_DPAD_DIGITAL

and for a build with Fn defined, could be replaced as

buttons GAMEPAD_MASK_S1
dpad    GAMEPAD_MASK_DOWN
aux     AUX_MASK_FN
action  HOTKEY_DPAD_DIGITAL

and so on from there.

This should yield a lot more flexibility in defining hotkeys, allowing users to customize what's what as makes sense to them, undefine modes they don't want, assigning them to different buttons (very weird buttons if they're so inclined), and even open up more actions (as they're introduced into the firmware) without us needing to touch the hotkey configs ourselves (unless defaults make sense to us).

This does not affect the S1 + S2 + Up BOOTSEL shortcut or the S1 web config mode shortcut at boot time.

[bsstephan]

Oh, and I just realized I never even looked at the LED hotkeys.
I think I covered the hotkeys now.

[TheTrainGoes]

Just wanted to mention this - there is an old thing buried in the code called the settings key.

You used to be able to have it assigned to a specific pin and it acted as the button that would need to be held to access shortcuts. The CrushCounter 2.0 used this.

We could look to re-implement this, however it comes at the cost of an additional pin.

[bsstephan]

On 2023-05-24 at 07:31:59 CDT, TheTrain ***@***.***) wrote: Just wanted to mention this - there is an old thing buried in the code called the settings key. You used to be able to have it assigned to a specific pin and it acted as the button that would need to be held to access shortcuts. The CrushCounter 2.0 used this. We could look to re-implement this, however it comes at the cost of an additional pin.

Oh! I will have to look for this when I'm back after the long weekend (I'm prepping for Combo Breaker now and will be gone until Tuesday). This implementation also uses up a pin, and a gamepad button, and the latter feels a bit wrong, so if there's some vestigial code that does this a bit cleaner, I will gladly pick it back up.

[bsstephan]

Bumped, last major thing to do is the web UI (API already working).

[bsstephan]

Oh! I will have to look for this when I'm back after the long weekend (I'm prepping for Combo Breaker now and will be gone until Tuesday). This implementation also uses up a pin, and a gamepad button, and the latter feels a bit wrong, so if there's some vestigial code that does this a bit cleaner, I will gladly pick it back up.

For the record I did start using the aux state and that removed the usage of a gamepad button.

[bsstephan]

This is done and ready for review and merge! Huge shoutouts again to @deeebug!

[arntsonl]

Some quick comments:

1. Should the fn's that are already assigned be disabled by default? Or is the enabled/disabled feature picked up by web config in my screenshot?
2. Things appear to get a little wonky when you assign more buttons on the right-side. The ordering changes (and do we have a depth we support/limit at?)
3. We need some kind of language to indicate what this is doing. I would say a good approach here would be a label in front of the Action (Action: Home Button Triggers: B2 B4 L1) or like (Home Button = B2 B4 L1).

The big thing is we're going to have users that don't really understand this feature. Some language either in the Hotkey Settings card would be awesome, or adding some qualifiers around the drop-down buttons would be great!

[arntsonl]

Also, if you add more keys to the shortcuts, you can't remove them. I'd be totally okay if we just limit this to 3 buttons

[bsstephan]

Some quick comments:

1. Should the fn's that are already assigned be disabled by default? Or is the enabled/disabled feature picked up by web config in my screenshot?

The "Fn" here (short for Function) is the new special button that no one will have wired, by default. The existing hotkeys don't use it, either (since it didn't exist), so it is off for them.

To your point below about better inline docs, this can probably be UXed better and I can work on that. Note that Fn isn't a gamepad button so it comes over the API separate from the rest currently. This could also be changed if it helps.

2. Things appear to get a little wonky when you assign more buttons on the right-side. The ordering changes (and do we have a depth we support/limit at?)

I noticed these issues as well. Probably the controls could be made less wide as a quick fix, but I don't know how to address the reordering that happens.

3. We need some kind of language to indicate what this is doing. I would say a good approach here would be a label in front of the Action (Action: Home Button Triggers: B2 B4 L1) or like (Home Button = B2 B4 L1).

debug was worried about some display issues with that but I can take a crack at it and see if anything can be done to illustrate the result better.

The big thing is we're going to have users that don't really understand this feature. Some language either in the Hotkey Settings card would be awesome, or adding some qualifiers around the drop-down buttons would be great!

As more people see this and get confused initially, I agree with you. The flexibility is going to go a long way, but it's kind of a lot to wrangle a UX for on the first go. Like I said though, I'll add more inline docs.

Probably worth mentioning that I think a really slick UI would be a graphical representation of the buttons (using the layouts?) and you click to activate the buttons you want for the hotkey, but I don't even know how one would begin to do that.

[bsstephan]

Also, if you add more keys to the shortcuts, you can't remove them. I'd be totally okay if we just limit this to 3 buttons

Selecting None in the dropdown removes it from the list. I thought it was clever. Maybe the docs explaining that will be enough, or maybe we can put a little X next to the dropdown, or maybe we just limit it to 3.

[bsstephan]

Oh yeah, while we're talking odd behaviors, it's worth noting that ordering of hotkeys matters in an unintuitive way, because the first hotkey to match will end processing. Example:

S1 + S2 + Up      = SOCD Up Priority
S1 + S2 + A1 + Up = Invert Y Axis

You'll never get Invert Y Axis to trigger, because even if you're holding A1, the rest of the first hotkey is triggered and fires. Swap them and you'll get both to be possible.

I think priority could be determined mathematically, but I haven't sat down and done the math to confirm it, and it would be a weird thing to process and renumber in the webconfig (but that'd be the right place for it, because it'd be a waste at runtime). Alternatively, we could see if people actually have this problem in practice. Or even more alternatively, we could change the hotkey matching to be exact matches (so that if you're holding a button not in a hotkey, it never fires, even if all the required buttons are held).

[mthiesen]

I think you need custom migration logic for your changes. You are just renaming hotkeyF1Up to a generic hotkey01 (the same is of course true for all the other hotkey settings).

People that have previously configured their hotkeys will expect their hotkeys to still work but the code will now evaluate the newly added buttonsMask and auxMask, which will be initialized to HOTKEY_XX_DPAD_MASK and HOTKEY_XX_AUX_MASK by initUnsetPropertiesWithDefaults(). These will not necessarily correspond to the previous fixed buttons as every board can configure this individually.

You can add your migration logic to ConfigUtils::load() before the call to initUnsetPropertiesWithDefaults() (not after as the comment suggests). You can evalutate the has_buttonMask flags of hotkye01 - hotkey08. They will only be set if the buttonMask was already present in the serialized data, which will not be the case when the config was written by a firmware version that did not know about buttonsMask.

[bsstephan]

I think you need custom migration logic for your changes. You are just renaming hotkeyF1Up to a generic hotkey01 (the same is of course true for all the other hotkey settings).

People that have previously configured their hotkeys will expect their hotkeys to still work but the code will now evaluate the newly added buttonsMask and auxMask, which will be initialized to HOTKEY_XX_DPAD_MASK and HOTKEY_XX_AUX_MASK by initUnsetPropertiesWithDefaults(). These will not necessarily correspond to the previous fixed buttons as every board can configure this individually.

You can add your migration logic to ConfigUtils::load() before the call to initUnsetPropertiesWithDefaults() (not after as the comment suggests). You can evalutate the has_buttonMask flags of hotkye01 - hotkey08. They will only be set if the buttonMask was already present in the serialized data, which will not be the case when the config was written by a firmware version that did not know about buttonsMask.

Thanks for the review and the heads up. I will look into this again when I get a chance, but I think you are right that there is a theoretical gap between old configs and the new HOTKEY_ defines. I think it'd only happen if anyone does make a board-specific set of defines before a current user of that board migrates to the new settings, but that is a plausible enough window that it's worth taking a look.

[bsstephan]

@mthiesen --- I don't think we need a specific migration, based on the behavior of ConfigUtils::initUnsetPropertiesWithDefaults --- as you said, right now it is setting hotkey0X.buttonsMask and hotkey0X.auxMask to the new values in GamepadConfig.h while retaining the previously-serialized hotkey0X.dpadMask and hotkey0X.action. However, no existing board config that we are responsible for made any changes to hotkey defines (hence their removal from the board configs), so they all follow the below general migration logic:

The values in GamepadConfig.h correspond to the old F1/F2 values. Specifically, HOTKEY_01_BUTTONS_MASK--HOTKEY_04_BUTTONS_MASK are S1 | S2 and 05--08 are S2 | A1. Additionally, the order of hotkeys 01--08 in the defines lines up with the previous order --- F1Up, F1Down, F1Left, F1Right, F2Up, F2Down, F2Left, F2Right --- in the protobuf config, meaning that old hotkeyF1Up's dpadMask and action are associated with HOTKEY_01_BUTTONS_MASK, and so on for the rest of the buttons.

As for auxMask, that is defined to 0 in all hotkeys, as it cannot be assumed that any previous board has a button wired to a pin for it. Thanks to debug's contributions it can't even be set to non-zero in the web configurator until Fn is given a pin mapping. There was no previous usage of aux before my PR, so no existing board can have an aux mask expectation.

With this logic, initUnsetPropertiesWithDefaults() does as expected, essentially taking the old hardcoded mappings and populating them into the config.

So, I don't think we have a problem in practice. As a further confirmation I just nuked a board, flashed it with main, changed a couple hotkey actions, confirmed they operate as configured, and upgraded to this branch --- the hotkey assignments were retained as expected, both in the web UI in the new style of hotkeys, and in function.

[bsstephan]

Added migration after the conversation on the discord. Tested by temporarily defining HOTKEY_XX_AUX_MASK = 1 on all 12 hotkeys in my board config --- after the migration, hotkeys 01--08 still had a mask of 0 (Fn off in the UI) (because they were migrated) and 09--12 had it on (because they had the board defaults populated via initUnsetPropertiesWithDefaults).

[arntsonl]

Looks great!!!