link vpot with corresponding display part

[markusschloesser]

while I was implementing group track fold/unfold and messing around with the display text, I thought it would come in handy if we could address the parts of a display which "belong" to the vpot underneath it. Something like vpot_encoder_display_index or similar

[markusschloesser]

a wait a second: just found vpot_index https://github.com/markusschloesser/MackieC4_P3/blob/d3b26386af8595f3afd611a41bca88db4d887ab5/wip/MackieC4/EncoderDisplaySegment.py#L61 in your encoderDisplaySegment 😂 will try to refactor the new fold/unfold to use your methods. Q: how does the set_text ((vpot_index), upper, lower) get along with all the hard coded strings in reassign and on_update?

[markusschloesser]

another q: you use vpot_index, but Encoders.py also uses vpot_index, but I think for the latter, it's the actual encoder whereas for you it's the display above the encoder, correct? If yes, I suggest to rename your's to vpot_index_display or so

[BilldarBagdar]

it's the actual encoder whereas for you it's the display above the encoder, correct?

Not really. The EncoderDisplaySegment class and the text it stores is about the display above the encoder for sure, but that specific class member is (supposed to be) a reference to the exact same "thing" as the member variable with the same name in "Encoders.py". The "encoder index". There are 32 of them on the device and each index anchors both a "display segment" and an "encoder". Or at least that's how I was picturing the arrangement.

I just used the same names that already existed, mostly for "continuity" at the first refactoring stage. Personally, I've never liked "vpot" as a name or name component at all. But initially, I didn't change it because I was trying to leave stuff mostly alone, and then I never got going down that rabbit hole. To me, "vpot" smacks of old-school c style naming conventions where the 'v' stands for the "data type" of the named variable like "vector" (C++ std lib). In the same style, "string names" start with 's', and "integer names" start with 'i'. Modern IDEs take care of that book-keeping for us now. I figure "pot" is short for "potentiometer", but why not "encoder"? I also figure maybe the decision came down to 4 letters versus 7. Either way, whatever, yes. Freely rename variables like that. ("encoder_index" maybe?)

But yes, part of the idea in the name "Encoder Display Segment" is the idea of only dealing with the 6 + 6 characters "always" available above any given encoder. Somehow I was picturing "EncoderController" itself getting much simpler and "Encoder" (We should drop the 's'. Plural means a collection, this class models one encoder) plus now "EncoderDisplaySegment" getting much smarter. Maybe each "Encoder" class object could start carrying around it's own "DisplaySegment", and the "EncoderController" just delegates and stops keeping track of some of those lower level details. But that's why I added those references back to the "associated encoder". They aren't necessarily used on the first pass where we just replaced the "display text tuples" that were so upsetting to the "on display update" method's tummy. Continue to feel free to refactor or add on or whatever you imagine. You have the ball with 25 meters to an open goal.

how does the set_text ((vpot_index), upper, lower) get along with all the hard coded strings in reassign and on_update?

Haven't tried that experiment yet. I left the hard coded stuff alone on the first pass, and then I wandered off into confusion about the repo. I'm also kind of waiting to see what gets ripped out and/or replaced, and where the problems are (if any) after that or on the way there. But to directly answer the question, strings are strings whether you display the ones hard coded directly in "on display update" or the ones hard coded into the "upper_text and lower_text" members of EncoderDisplaySegment (in "reassign encoder parameters"). The only problem there I suspect might be the Mute On/Off display would get out of sync with Live's Mute On/Off status. The "right" way to fix the "out of sync" problem would be along the "mute listener" path somewhere. The code in "on display update" that queries Live (10 times a second) and also fixes the problem appears to be more of a "just make it work and move on" fix. (In other words, haven't tried because it might be a can of worms to fix correctly that could change anyway depending on what your changes accomplish...)

Ultimately, if we want to go that far, there's no reason we couldn't address the LCD display text like cells of memory, I mean directly address each "character" of LCD display. (So you could, for example, maybe use '|' (pipe character) to mark "sub sections" of controllers on a row, or write one word over 4 related encoders (Stuff C4 Commander layouts can do) 7 characters over an encoder * 8 encoders on a row means 56 characters on a row. But each LCD actually has IIRC 63 characters of space available and the "extras" can only really effectively be used by "direct address".

[markusschloesser]

Continuing this discussion (cos I was researching a lot) and picking up on what you wrote: Ideally (imho) we should have an encoder class which carries around ALL things related to the encoder, so

1. two display segments(upper, lower)
2. the encoder itself consisting of a) vpot_pressed b) vpot_rotate
3. the encoder ring In order to not implement new stuff in 4 different places

I researched what is already there in the framework classes for that (posting how other scripts are using that):

for 1: https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/ableton/v2/control_surface/elements/physical_display.py implemented in e.g. https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/SL_MkIII/elements.py , however they use their own intermediate super class which imports the v2 framework class https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/SL_MkIII/physical_display.py

for 2 a: https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/ableton/v2/control_surface/control/encoder.py implemented in (I think) https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/d77ab10c837503adea80801c20c92c12d6be245a/Push2/push2.py#L1136

for 2 b: encoder.py as 2a above and 3 below

for 3: https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/KeyLab_Essential/ringed_encoder.py and https://github.com/markusschloesser/AbletonRemoteScriptsPY/blob/main/KeyLab_Essential/ringed_mapped_encoder_control.py

All of these bring proper modern functionality with them (no need for self built listeners)

So the questions are: Could you do that encoder class? Does that even make sense? ;-) Does it make sense to do that in our script, or do we need to start fresh? This question keeps popping up in the last weeks. Also: Do you know of a way to find out which parts of our script are actually used? I suspect there might be quite a bit not used at all

[markusschloesser]

to combine things from different classes, we need to use the CompoundElement class from ableton\v2\control_surface\compound_element.py. The above example for the ringed_encoder does exactly that.

I've added the docstrings for that CompundElement and other ControlSurface classes in the other repo