Cipulot refactoring

[Cipulot]

Description

Refactoring of the EC code used in my boards.
After the suggestion on previous PR for some of my boards and discussion in the QMK server I've managed to consolidate the EC code I use under a common folder under keyboards/cipulot.
This change was initialized in #21233 with the move of the custom via_apc.c code used for some custom handling of actuation points etc.

The code refactoring includes the following enhancements:

• removal of ec_switch_matrixc, ec_switch_matrix.h and matrix.c from each keyboard folder and put it under keyboards/cipulot/common
• Replaced via_apc.c with via_ec.c for additional features
• Disabled bootmagic (was causing issues) and placed a dummy void bootmagic_lite(void) function that ensures that VIA doesn't re-enable it when in use.
• Addition of APC and Rapid trigger for all EC boards (controllable through the VIA UI)
• Addition of a floor noise sensing routine at boot (also manually operable in case boot run provides wrong results)
• Addition of a bottom-out calibration routine
• Definition of onboard memory that stores essential information for persistency
• Change in save behavior when handling VIA IDs so that EEPROM doesn't get written at each new data, but rather it gets written only after user confirmation
• Addition of OPT flags to reduce space usage and increase scan rate (tested, OPT flags differ from one board to the other to mitigate bugs in RGB using ARM)
• Fixing #include in the files to account for changes
• Addition of #define VIA_FIRMWARE_VERSION 1 in VIA keymap to avoid conflicts in menu fetching with boards with an older version

This refactoring allows the future boards I'll be merging to use shared code as it's supposed to be, that is by use of #include and not code cloning.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[yulei]

1. if the keys were pressed while plugging the usb cable, the noise floor will get bad data.
2. in the RT implementation, it's risky to just use the current data compared with previous data because you can't garentee the stability of the ADC results. It will have random noise from other part, and the ADC timing also affects the ADC result because the time interval between the pin strobe and ADC start was not same at every time e.g. the interrupt occured randomly.

[Cipulot]

@yulei

1. if the keys were pressed while plugging the usb cable, the noise floor will get bad data.
2. in the RT implementation, it's risky to just use the current data compared with previous data because you can't garentee the stability of the ADC results. It will have random noise from other part, and the ADC timing also affects the ADC result because the time interval between the pin strobe and ADC start was not same at every time e.g. the interrupt occured randomly.

1. For the first one there's really no other way to scan the floor noise other than ask the user to not press them keys while that is running. A way to mitigate that is to force a noise floor update with a button in VIA, but this still counts on the user not pressing. More than putting a warning can't be done much
2. For that, since proper interrupt disable isn't implemented correctly (ADC chibios bypasses the locks for whatever reason), the simplest solution w/o investigating the deep dives of chibios doodo code is to add those release/actuation sensitivity values, creating a hysteresis window that mitigates that behavior

[yulei]

@yulei

1. if the keys were pressed while plugging the usb cable, the noise floor will get bad data.
2. in the RT implementation, it's risky to just use the current data compared with previous data because you can't garentee the stability of the ADC results. It will have random noise from other part, and the ADC timing also affects the ADC result because the time interval between the pin strobe and ADC start was not same at every time e.g. the interrupt occured randomly.

1. For the first one there's really no other way to scan the floor noise other than ask the user to not press them keys while that is running. A way to mitigate that is to force a noise floor update with a button in VIA, but this still counts on the user not pressing. More than putting a warning can't be done much
2. For that, since proper interrupt disable isn't implemented correctly (ADC chibios bypasses the locks for whatever reason), the simplest solution w/o investigating the deep dives of chibios doodo code is to add those release/actuation sensitivity values, creating a hysteresis window that mitigates that behavior

for 1, pre-set data can be used while found the ADC data were not as expected. the noise data should be update on every iteration if the data looks good, not only at startup. for 2, some ADC data should be dropped if they looks strange. Not every ADC data can be used during matrix scan and they may lead to unexpect issues especially under RTOS such as chibios bedcause it's time critical.

[Cipulot]

Updating the noise floor data on every iteration is pointless, since unless the board is modified while it's powered, the average noise floor doesn't deviate in any noticeable way. It is a good option if scan rates aren't affected in a meaningful way, but they are heavily impacted by basically any operations. A possible mitigation might be checking if during the N amount of noise floor "gathering" function, if the reading is above let's say 3/4 of the available scale, then it gets disregarded. And if the entire sample for that key/keys are disregarded they are automatically supposed to be either 0 (ideal no-press) or the same as the average of the rest of the keys, just because it's usually how the noise behaves if the board is properly made.
For the later observation, if abnormal readings are detected then there's a deeper issue with the board design. Mitigation of bad ADC readings could be implemented by bypassing the default ADC_SAMPLING_RATE and choosing a parameter that better matches the electrical characteristic of the signal path.

[Cipulot]

For the point 1 I've decided to default floor noise to 0 if the reading is above VALUE. Also will be adding an additional "Get noise floor" or something like that button that gets the floor noise at runtime.

For the actual VALUE I'll have to test which one makes more sense to use.
Main reason why this value isn't easily determinable is that it's not really possible to distinguish beyond a reasonable doubt if a bad reading, aka a value that deviates off of 0 + real noise, is caused by the user having keys pressed or for other reasons.
Some of them might include:

• badly aligned springs/domes that cause a pre-compression
• poorly designed board with either wrong analog gain settings
  In all those cases disregarding the values at boot is problematic, to mitigate the errors that might be introduced because of involuntary presses I've added a button that calls the function again at will through VIA gui. In there a dedicated text will remember the user to not press any key while clicking the button.
  this will ensure that if any wrong floor noise is detected it's going to be either bad mounting or bad PCB, things that in both cases are not firmware fixable.

[drashna]

2. For that, since proper interrupt disable isn't implemented correctly (ADC chibios bypasses the locks for whatever reason), the simplest solution w/o investigating the deep dives of chibios doodo code is to add those release/actuation sensitivity values, creating a hysteresis window that mitigates that behavior

You mean the chSysLock/chSysUnlock calls?

[Cipulot]

2. For that, since proper interrupt disable isn't implemented correctly (ADC chibios bypasses the locks for whatever reason), the simplest solution w/o investigating the deep dives of chibios doodo code is to add those release/actuation sensitivity values, creating a hysteresis window that mitigates that behavior

You mean the chSysLock/chSysUnlock calls?

@drashna I gotta check the convo that I had with sigprof where we discussed about it. iirc it had something to do with those yeah

[drashna]

That's ... a rabbit hole, for sure.

[Cipulot]

Yeah, one that for now isn't causing any issues. Of course for correctness that should be handled but as of now I have already some work in the pipeline and entering the matrix for that isn't a priority.
I can add a warning along with some info on how to set things up in a markdown file that I can add in the 'common' folder for that tbh.

[Cipulot]

It appears that the recent merge of #22448 causes the conflicts. Since prior to that merge the inclusion of analog.c was explicit in rules.mk while this PR requires the use of ANALOG_DRIVER_REQUIRED = yes.

[Cipulot]

@drashna could you please take a look at the conflicts and mark them as solved, I applied a patch to align the inclusion of the analog.c using the latest guidelines merged in the commit be45346.
Thanks :)

[elpekenin]

No idea about the actual implementation of scanning code, but assuming you tested it.

Just a couple of cleanup/enhancement ideas from my end.

BTW, GitHub showed something like "things need to be resolved", unsure if you have some merge conflict or just some "requested changes" pending

PS Didnt look at keyboards' code, just an overview of the common folder

[Cipulot]

No idea about the actual implementation of scanning code, but assuming you tested it.

Just a couple of cleanup/enhancement ideas from my end.

BTW, GitHub showed something like "things need to be resolved", unsure if you have some merge conflict or just some "requested changes" pending

PS Didnt look at keyboards' code, just an overview of the common folder

For the conflicts, I applied the new inclusion of analog.c in the latest commit, tho for some reason, the conflicts are still flagged. Not having writing perms on the PR I can't mark them as solved tho.

[keyboard-magpie]

Have done my best at testing this by pulling in the firmware files for ssbb/tako. All appears to work well, to the best of my testing ability.