OpenStickCommunity / GP2040-CE

Multi-Platform Gamepad Firmware for Raspberry Pi Pico and other RP2040 boards
https://gp2040-ce.info
MIT License
1.4k stars 310 forks source link

Profile improvements roadmap #1123

Open bsstephan opened 1 month ago

bsstephan commented 1 month ago

At the risk of sounding like I'm setting something in stone, I thought I'd write down my thoughts for recent profile expansion conversations to talk through it.

  1. Finish #807 --- I have internals in a draft, and a PR from Pelsin to implement the UI, and then I just need to test OLED display a bit.
  2. Add profile enabled field --- seems like a good idea for a toggle profile hotkey. Would default to enabled for backwards compatibility, no need for a migration. I think this would go in GpioMappings along with the label.
  3. Consolidate the core Config GpioMappings and ProfileOptions GpioMappings, making the former just one more entry in the latter. This cleans up some code, and especially removes some duplication in the API and UI code. I always meant to do this, and there might be some landmines here to look into, but this seems like the right choice. The migration for this would bump ProfileOptions.gpioMappingsSets from 3 to 4, copy 3 into 4, 2 into 3, 1 into 2, Config.GpioMappings into 1, and then deprecate Config.gpioMappings. Thankfully there's already a bit of a facade pattern for this thing so I think most of the gamepad code won't need to be changed. While I'm here, I should make sure nothing hardcodes 3/4.
  4. By this point I probably need to have increased the Config size, but if I somehow haven't... maybe we should just do it here anyway, in anticipation of:
  5. Bump profiles from 4 to 6. If I do item 3 right, this is either a protobuf change and updating one define, or maybe just the protobuf change.
Pelsin commented 1 month ago

A thought that occurred. Any input can be done for the profile label, e.g "ÅÖÄ". Currently it works nicely in the UI, but i assume there could be issues rendering these chars on the display.

mikepparks commented 1 month ago

A thought that occurred. Any input can be done for the profile label, e.g "ÅÖÄ". Currently it works nicely in the UI, but i assume there could be issues rendering these chars on the display.

Currently high ASCII is reserved for extra glyphs with input history, and doesn't account for accented characters. There's also a concern about Unicode, which is pretty much no-go with the current font set.

bsstephan commented 1 month ago

How difficult is it to constrain the UI fields to low ASCII? A protobuf string of len X translates to a char array of len X+1.

mikepparks commented 1 month ago

For the sake of profile names? Trivial.

Pelsin commented 1 month ago

So adding validation for the input between the \x20-\x7F range? Should be simple yeah, I'll update the PR when the time presents itself.

arntsonl commented 3 weeks ago

For #3, getting gpio mappings, I think we can be profile agnostic and wrap a function around it. So for example:

instead of: mappingUp = profileMapping[currentProfile][up];

we do: mappingUp = mapping->getPin(up); or mappingUp = getMapping(up);

Then its easier to implement different ways to handle the way we get up, etc. That way if we lets say add more profiles, or add some checking logic, etc. etc. its all broken out into its own functionality. Basically wrap a tiny API around the functionality so its easier to expand or change logic in the future.

Memory size is getting pretty big, I think its a good time to think about board settings vs user settings protobuf packages. We're at the point where calling protobuf breaks our core1 usb code, so we might want to think deeper on how to break up those two components.

So user settings MUST be under a certain size to ensure we don't break usb host, and board settings can be some big size.

Pelsin commented 3 weeks ago

Updated the PR Sep-04-2024 20-42-50

bsstephan commented 1 week ago

For #3, getting gpio mappings, I think we can be profile agnostic and wrap a function around it. So for example:

instead of: mappingUp = profileMapping[currentProfile][up];

we do: mappingUp = mapping->getPin(up); or mappingUp = getMapping(up);

Then its easier to implement different ways to handle the way we get up, etc. That way if we lets say add more profiles, or add some checking logic, etc. etc. its all broken out into its own functionality. Basically wrap a tiny API around the functionality so its easier to expand or change logic in the future.

Memory size is getting pretty big, I think its a good time to think about board settings vs user settings protobuf packages. We're at the point where calling protobuf breaks our core1 usb code, so we might want to think deeper on how to break up those two components.

So user settings MUST be under a certain size to ensure we don't break usb host, and board settings can be some big size.

The current method of loading the profile into an in-memory mapping structure should satisfy the main vs. alternative concern, though I agree that the "API" of it could be enhanced or improved. I cheated with the profile labels and just added a method to storage to get it in an agnostic fashion regardless of the copied live mapping, which works but isn't unified in the way you might be thinking.

As for protobuf(s), I think that conversation might be a bigger thing to chew on.

bsstephan commented 4 hours ago

Update: I scratched out step 3 (combining GpioMappings into one area) because I think there is still some logic, in how step 2 turned out, in keeping them separate for now, so it's mostly a deferment until/unless it becomes a problem.