Implement generalized analog input module

[xs5871]

Draft of a better analog input module as mentioned in #638.

[piman13]

so AnalogIn is being imported twice in this exsample
I think its suppose to be from kmk.modules.analogin import AnalogInput, AnalogKey

import board
from analogio import AnalogIn
from kmk.modules.analogin import AnalogIn

Other note where is this suppose to be placed
or is this my lack of understanding of kmk

you import the stuff above in code.py
but KC is not avable in code.py
putting it in keymap.py would make sense so are the imports suppose to be there as well?

analog = AnalogIn(
    [
        AnalogInput(AnalogIn(board.A0)),
        AnalogInput(AnalogIn(board.A1)),
        AnalogInput(AnalogIn(board.A2)),
    ],
    [
        [AnalogKey(KC.X), AnalogKey(KC.Y), AnalogKey(KC.Z)],
        [KC.TRNS, KC.NO, AnalogKey(KC.W, threshold=96)],
    ],
)

keyboard.modules.append(analog)

[piman13]

oh and is the analog = AnalogIn suppose to be AnalogInputs as well?

analog = AnalogIn(
    [
        AnalogInput(AnalogIn(board.A0)),
        AnalogInput(AnalogIn(board.A1)),
        AnalogInput(AnalogIn(board.A2)),
    ],
    [
        [AnalogKey(KC.X), AnalogKey(KC.Y), AnalogKey(KC.Z)],
        [KC.TRNS, KC.NO, AnalogKey(KC.W, threshold=96)],
    ],
)

edit "AnalogInputs" not AnalogInput

[piman13]

also a minimum change for on_change needs to be added either as a override-able value or just a 1 or 2% min-max (pots like to float a little and hopping between 1-10 units all the time keeps triggering it

[piman13]

question... I've added pulls to the analogin branch does this need redone or if you merge it it will the draft update?
other note. so much better to work with compared to potentiometer.py and I'm working on example code for calling midi to set the volume of stuff on the computer

Now is there any good way to pass a int to this so we don't have to copy pasta what is effectively the same code
Something like SDL(0)

SLD0 = AnalogEvent(
    on_change=lambda self, event, keyboard : KC.MIDI_CC(7, event.value, channel=0).on_press(keyboard),
)
SLD1 = AnalogEvent(
    on_change=lambda self, event, keyboard : KC.MIDI_CC(7, event.value, channel=1).on_press(keyboard),
)
SLD2 = AnalogEvent(
    on_change=lambda self, event, keyboard : KC.MIDI_CC(7, event.value, channel=2).on_press(keyboard),
)
SLD3 = AnalogEvent(
    on_change=lambda self, event, keyboard : KC.MIDI_CC(7, event.value, channel=3).on_press(keyboard),
)

[xs5871]

@piman13

so AnalogIn is being imported twice in this exsample I think its suppose to be from kmk.modules.analogin import AnalogInput, AnalogKey

I had to change all the names midway through, because I didn't notice that CP already uses the name analogin.
Docs are fixed now

Other note where is this suppose to be placed or is this my lack of understanding of kmk

you import the stuff above in code.py but KC is not avable in code.py putting it in keymap.py would make sense so are the imports suppose to be there as well?

Yes, you have to import KC. The docs show examples, they don't spell out a complete configuration.

also a minimum change for on_change needs to be added either as a override-able value or just a 1 or 2% min-max (pots like to float a little and hopping between 1-10 units all the time keeps triggering it

1. That's what the filter argument is for.
2. The default filter already downscales to 8bit. Most MCUs have 12bit ADC, so that's a noise reduction factor of 16. In my experience that is enough in pretty much any reasonably designed circuit. If your ADC measurements still vary by 10 units, then that is definitely a hardware/design issue. Check your ADCs input impedance requirements.

Now is there any good way to pass a int to this so we don't have to copy pasta what is effectively the same code
Something like SDL(0)

Why use midi keys in the first place? Just use the midi API directly.

[xs5871]

Are there any fundamental issues within the scope of this PR left unaddressed, or can we merge?

[claycooper]

LGTM

[piman13]

Question, was the original plan to make some modules that need/ provide some use for this first so it won't sit without use like pot did originally
also I almost have a new version done that supports the stuff discussed in zulip, specifically this scope here
I don't think there's to much wrong with the original but it lacks simplicity for the user and relies on lambda being a norm

for a hall effect keyboard there should be no reason to not be able to just add a normal keymap and it makes the map in the backend.

also speaking of hall effects. this won't work with layer keys. nothing is done when swapping layers to set the state back and it remains locked on. same for other keys

[xs5871]

Question, was the original plan to make some modules that need/ provide some use for this first.

No. Why are we having this discussion for the n-th time?
We are solving problems, fixing issues, implementing features as they present. No single commit is supposed or intended to cover every eventuality and the kitchen sink. The potentiometer module is a bad framework for further work and it lacks documentation: here's the fix. That does not mean this implementation is set in stone.

so it won't sit without use like pot did originally also I almost have a new version done that supports the stuff discussed in zulip, specifically this scope here

That discussion has evolved into pages upon pages of unfocused rubber ducking. That is fine, that's kind of what the chat is for, in a sense. It does not, however, have any tangible "scope".

I don't think there's to much wrong with the original but it lacks simplicity for the user and relies on lambda being a norm

No it doesn't. We've been over this.

for a hall effect keyboard there should be no reason to not be able to just add a normal keymap and it makes the map in the backend.

also speaking of hall effects. this won't work with layer keys. nothing is done when swapping layers to set the state back and it remains locked on. same for other keys

Modules are mainly for peripherals. A hall effect keyboard would need a specific scanner in order to work with the rest of KMK. Once again, this is not the intendend use of this module and out of scope for this PR.

[piman13]

Ok so just thinking about the peripherals then. they will press keys, someone changes a layer for other macros and its in an active state.
it keeps pressed

filters are currently still locked to the input and not the event meaning a midi pitch bend on one layer and a joystick on another wont work without the midi making sure the value is down sampled

say you make a midi keyboard and you swap layers one the fly for different instruments or groupings. your midi key is stuck until you go back.

say its a joystick being uses as a wasd controller (legit had someone ask about that at one point)
with the examples in the analogin.md doc. I legit have no idea how you would make a "JoyKey"

this is all that's needed when the other structures are there
simple readable and layer safe.

#----keymin-enabled--------both-disabled---------keymax-enabled
#------------thresmin--------deadzone-------thresmax-------#analog-max
class AnalogJoyKey(AnalogEvent):
    def __init__(self, keymin, keymax, thresmin, thresmax):
        self.keymin   = keymin
        self.keymax   = keymax
        self.thresmin = thresmin
        self.thresmax = thresmax
        self.minpressed = False
        self.maxpressed = False

    def on_change(self, event, keyboard):
        if event.value >= self.thresmax and not self.maxpressed:
            self.maxpressed = True
            keyboard.pre_process_key(self.keymax, True)
        elif event.value <= self.thresmax and self.maxpressed:
            self.maxpressed = False
            keyboard.pre_process_key(self.keymax, False)

        if event.value <= self.thresmin and not self.minpressed:
            self.minpressed = True
            keyboard.pre_process_key(self.keymin, True)
        elif event.value >= self.thresmin and self.minpressed:
            self.minpressed = False
            keyboard.pre_process_key(self.keymin, False)

    def on_stop(self, event, keyboard):
        pass

    def layer_change(self, event, keyboard, ingress):
        if ingress:
            self.on_change(event, keyboard)
        else: #egress
            self.minpressed = False
            keyboard.pre_process_key(self.keymin, False)
            self.maxpressed = False
            keyboard.pre_process_key(self.keymax, False)

think about how user will use it and what they want to use it for,
not everyone is a python dev and I'm no expert on python either

but seeing this
a0 = AnalogInput(dac, lambda _: int(_.value / 0xFFFF * 1980) + 20)
when I didn't know as much.
had me confused about what it did with that and how it worked, I didn't even want to use it until learned more python and how lambdas worked

We are solving problems, fixing issues, implementing features as they present. No single commit is supposed or intended to cover every eventuality and the kitchen sink. The potentiometer module is a bad framework for further work and it lacks documentation: here's the fix. That does not mean this implementation is set in stone.

right but you said it with encoder. it went bad because people kept wanting other functionality out of it and it got bloated

it needs to have a way of providing "almost" everything a module or user could need or the resources to form that need (eg timedelta for velocity calculations)

And if were missing something it should not result in a format change for existing users and modules.

I'm trying to look at this as if I were to give my friend who knows a little programing. the module and the docs and to tell them to make something. and asking what they would want to make

[xs5871]

Ok so just thinking about the peripherals then. they will press keys, someone changes a layer for other macros and its in an active state. it keeps pressed

It's a toggle. You change layers, it stays toggled. Literally intended behaviour, exactly the same as toggles in the main keymap are expected to work.

filters are currently still locked to the input and not the event meaning a midi pitch bend on one layer and a joystick on another wont work without the midi making sure the value is down sampled

I'm getting really tired of repeating myself. The filters are for normalizing the analog readings. They are supposed to be independend of any downstream actions.

say you make a midi keyboard and you swap layers one the fly for different instruments or groupings. your midi key is stuck until you go back.

Same as the first: If I don't "release" a key intentionally, I'd expect it to stay active until I do so.
If that doesn't match your

say its a joystick being uses as a wasd controller (legit had someone ask about that at one point) with the examples in the analogin.md doc. I legit have no idea how you would make a "JoyKey"

Ok. So? Is you having no idea a technical limitation of the framework?
We can add a two way AnalogKey. That's not even particularly difficult. I'll do you one better: let's make it a four way AnalogKey that adds shift for sprinting if it exceeds a certain threshold.
How often do I have to repeat that a first implementation, that is explicitely meant as a basis for extensibility, will not come with every concievable gimmick you can think of. Adding things on demand is trivial if you plan for it.

think about how user will use it and what they want to use it for, not everyone is a python dev and I'm no expert on python either

but seeing this a0 = AnalogInput(dac, lambda _: int(_.value / 0xFFFF * 1980) + 20) when I didn't know as much. had me confused about what it did with that and how it worked, I didn't even want to use it until learned more python and how lambdas worked

There's a description of what it does in plain english right above the code snippet.
There are multiple examples in the doc that progressively get more complicated. You could say: there's an example for every level of code literacy.
This is a code project. You necessarily have to read and write code to use it. If you don't understand something, you look it up.

We are solving problems, fixing issues, implementing features as they present. No single commit is supposed or intended to cover every eventuality and the kitchen sink. The potentiometer module is a bad framework for further work and it lacks documentation: here's the fix. That does not mean this implementation is set in stone.

right but you said it with encoder. it went bad because people kept wanting other functionality out of it and it got bloated

it needs to have a way of providing "almost" everything a module or user could need or the resources to form that need (eg timedelta for velocity calculations)

And if were missing something it should not result in a format change for existing users and modules.

That's not what I said. I said it was poorly designed from the start, especially without extensibility in mind.
Starting with a minimal basis doesn't mean it gets bloated when you add functionality, if that funcionality is modular and optional.
You can plan and design for future improvements. There's a difference between planning and cramming. The encoder module didn't, it just got crammed full of dependencies. Like a "velocity" that no one actually uses. If at some point you come up with an event handler that needs that, feel free to implement it there.
It does not have to provide almost everything (that's called bloat), it's supposed to facilitate almost everything without bogging down the simplest use case: just a pot to turn down the gamer lights. Which it pretty much does.
KMK is software under active development and is far from being considered "stable". It will have breaking changes, as software invariable does. We do keep breaking changes to user-facing parts to an absolute minimum. You don't have to tell us.

I'm trying to look at this as if I were to give my friend who knows a little programing. the module and the docs and to tell them to make something. and asking what they would want to make

Cool. You can use their input to suggest neat new ways to use it that we haven't thought of yet.