Closed markusschloesser closed 1 year ago
copy and paste of commit note here: Grouped/racked devices can now be accessed as well as normal devices. The group is flattened and shows the params for the group device (macros) and params for all devices that are part of the group.
Extensive logging still enabled, should there be a problem. The solution DOES NOT do the proposed folding/unfolding of chains/racks/groups, but instead just gets and shows the whole list of devices: TODO: Update documentation
Nice work. I too am familiar with adding tons of logging to figure out exactly what's going on. I also noticed a question in a comment that was part of your a21cc76 commit.
[0 for i in range(SETUP_DB_DEFAULT_SIZE)] # @Whiner: why is the "i" not used?
Do you know the Python phrase "List Comprehension"? I don't really understand what the word "comprehension" means in this context, but I think of "List Comprehension" as "List Builder syntax".
The decision about "using" the i
value is completely determined by the content a programmer wants in the constructed List. In the case above, the constructed List will be length 128 and there will be a 0 value at every index. Here's two other "List Comprehension" examples from my recent coding escapades:
[display_mode_min_value + x for x in range(feedback_val_range_len)]
[display_mode_min_value if x < halfway else display_mode_max_value for x in range(feedback_val_range_len)]
The first creates a normal "encoder feedback value map" (List) and the second creates a boolean "encoder feedback value map" of the same length. The x
is used in both cases to determine the length of the constructed List, but the value of the x
is only used in the first case for the constructed values in the List, the x
value is only used logically in the second case.
Thanks! That was a big one with many wrong turns taken. Right now it is built on an answer I received from a very generous soul in the Ableton forums, but there is some similar method that I already integrated last year in track utility which I tried to do but which didn't work, so I either throw that additional functionality out at some point or I'm switching as well. I also need to do some general clean up at some point. So if you get a chance please do test this extensively especially as I am messing around with the index /had to change a lot about the indexing of the device list. Especially the undo and redo functionality, as always, kept throwing errors. The other thing is that all the build up of the setup database is done in encoder controller, except for the master devices which you for some reason put into the the encoder assignment history. Could you you if you have the time elaborate why you did it that way? Also thanks for the i explanation! Appreciated!
Ok, there's a bug. I'll try to explain: I can group and ungroup, group groups, etc. BUT: When I pull 2 group devices on a track and THEN undo OR delete the second group device, 2 things happen:
RemoteScriptError: cb = lambda: self.device_changestate(track, tid, type)
RemoteScriptError: File "C:\ProgramData\Ableton\Live 11.1\Resources\MIDI Remote Scripts\MackieC4_DEV\MackieC4.py", line 834, in device_changestate
RemoteScriptError: self.__encoder_controller.device_added_deleted_or_changed(track, tid, type)
RemoteScriptError: File "C:\ProgramData\Ableton\Live 11.1\Resources\MIDI Remote Scripts\MackieC4_DEV\EncoderController.py", line 285, in device_added_deleted_or_changed
RemoteScriptError: updated_idx = self.__eah.device_added_deleted_or_changed(extended_device_list, self.selected_track.view.selected_device, selected_device_idx)
RemoteScriptError: File "C:\ProgramData\Ableton\Live 11.1\Resources\MIDI Remote Scripts\MackieC4_DEV\EncoderAssignmentHistory.py", line 365, in device_added_deleted_or_changed
RemoteScriptError: assert new_device_count_track == self.t_d_count[self.t_current]
RemoteScriptError: AssertionError
The log lines BEFORE that assertion error show:
(MackieC4) C4Comp: track <1-Audio> tidx <0> type <0>
(MackieC4) EC: processing device changestate of type <0> on track <1-Audio> at index 0
(MackieC4) EAH: track device list size <3> BEFORE device update
(MackieC4) idx <0> before
IMHO if I understand those log lines (from `device_added_deleted_or_changed` in EncoderController and from EAH) correctly, things SHOULD be fine, but...
2. The devices list in chan strip mode row 2 still shows the old/wrong device count (6) and lists the (deleted) devices and their names.
3. Switching to Track mode shows "No Devices selected on this track", which is not true, Live shows the remaining group device is selected. Switching back to ChanStrip, then shows the correct amount of devices (3). So the `handle_bank_switch_ids` function works. `handle_slot_nav_switch_ids` also works fine, I cannot select a no longer existing device.
4. redoing this without 3. but instead selecting via vpot_push the
I. first (macro) device > works fine
II. second or third device (in group) > works but shows same assertion error as above
III. selecting one of the deleted devices > shows "No Devices selected on this track" (untrue) and assertion error
5. again redoing 1. and then selecting one of the still existing devices in Live with the mouse, doesn't change anything, BUT a second selection of another device, then cleans up the (deleted) devices π€―
I am not sure, if this is an index problem or a "device selection" problem, any ideas?
TBH I still don't fully understand your EAH class, but especially" suspicious for me right now is
1. `devices_on_track = tracks_in_song[t_idx].devices` (line 105). Not sure, if this needs a `self.get_device_list` for the devices part?
2. The other candidate is the `device_added_deleted_or_changed` in EAH, lines 270 and ff, where the last (non-error) log lines from above come from.
3. Third candidate is obviously EAH lines 343 and ff (`elif device_was_removed:`)
Or are the assertion error and the other actual error 2 different unrelated errors?
to answer my own question, the 2 errors are indeed IMHO unrelated! If I undo a RackDevice insert on the Master or other tracks, there's no assertion error, but the Devices row in ChanStrip mode doesn't update. Removing (instead of undoing insertion), works correctly. Clicking on vpot push for a no longer existing device throws an IndexError (of course).
Update 1: just checked, this also happens when undoing insertion of a non-rack device (a normal device). mmh was it always like this? π€ Will check Update 2: yes, older versions are also affected. So at least I didn't cause this ππ Update 3: So why is undoing device insertion different than deleting? Any why don't we get notified? Also this might be unrelated to the problem above, because this error does not happen when deleting a device, whereas the error from above happens in both cases.
I think something is amiss in line 1374 in EncoderController text_for_display = next(x for x in self.__display_parameters if (x.filter_index(t)))
because this doesn't update the Devices list properly
or
lines 1035ff elif s_index in row_01_encoders:
For the suspected line 1374 error, trying to put in lower_string1 += adjust_string(self.selected_track.view.selected_device.name, 6)
doesn't work. Shows no devices at all.
Then we have all the dlisten (DeviceListener) stuff in MackieC4, which I never really understood and still don't do
to summarize:
device_added_deleted_or_changed
does NOT get triggered)possible faults and explanations (besides what already mentioned above):
dlisten
in MackieC4 (index?), add_devicelistener
(with the ugly lambda calling device_changestate
), add_device_listeners
(plural), do_add_device_listeners
add_selected_chain_listener
, though I'm not sure about this, because this sounds like listening to changes IN the rackdevice and not WITH the rackdevice. Unfortunately, except Push, NO SCRIPT uses or can access rack/chain devices. So not much info to check for me.I am not sure, if this is an index problem or a "device selection" problem, any ideas? TBH I still don't fully understand your EAH class, but...
You/We should be able to go back and review the original "EAH class" commits to be sure, but as I recall, the goal of the "encoder assignment history" class was simply to move that code out of the "EncoderController" class where it was "visually / cognitively polluting" our understanding of how the LCD text was getting updated. In other words, all the code in "EAH" should be functionally equivalent to the same code in "EncoderController" before the code moved to "EAH". Which means, IMHO, this bug is "architectural" versus being the result of a typo or something easy to fix.
The other thing is that all the build up of the setup database is done in encoder controller, except for the master devices which you for some reason put into the the encoder assignment history. Could you you if you have the time elaborate why you did it that way?
Again, all I tried to do was move the existing "assignment history" functionality out of "EncoderController" where it obscured "encoder control" functionality into it's own class where it couldn't obscure any other code. The "master devices" are set up how and where they are because that's how the original code did it. As I recall, the "EAH database" allows for 128 "top level tracks" in a song arranged in the order "regular tracks", then "return tracks", then "master track". The reason the "master track" setup occurs later is because every "regular track" and "return track" has to be set up properly first so the "master track index" ("last return track index + 1") will be known.
I haven't actually done any research, this is speculation. But, I suspect the problem is related to "recursiveness" and the fact that the original "encoder assignment history database" was not designed with "recursive flexibility". What I mean is all of the lists that comprise the "EAH database" are fixed at length 128, and the database itself has no concept of "device racks". What I mean is, at whatever level in the "EAH database" there is a "db record" representing the index of the "selected device" in the "selected track's device chain". But when "device racks" are involved, that "index" needs to recursively account for all nested devices if any exist.
All the clues you've outlined lead me to SWAG whatever changes you made are logically correct in the forward direction, but some logic remains missing in the "cleanup direction". Thinking about what happens to the "master track index" when grouped "regular tracks" are opened/closed may be instructive in the devices and/or racks added/removed and/or opened/closed cases.
Conceptually, the evidence you present seems to describe a problem that seems very similar to correctly showing/hiding grouped tracks. At the (top) "tracks" level (in the EAH db) , the "master track index" changes by the size of the "track group" every time the show/hide grouped tracks view changes.
For example, given a song with 1 (regular) group track (grouping 4 nested tracks), and 0 return tracks;
I suspect the problem you are describing is in that same class of bugs issues. When the view changes between showing and hiding the grouped tracks, the size of the running "functional" database (list) needs to shrink by the same number of grouped tracks (the same amount it expanded). I.E. the "master track index" (db record) needs to move back to index 1 from index 5 when the grouped tracks are hidden again.
Your changes are likely correctly moving the "selected device" index from x to x + n to "expand" the "device_list" view, but not yet correctly moving the index from X + n back to x to "shrink" the view. In other words, your code changes are likely "expanding" the number of "database records" at the "device list" (index of selected device?) "table" level correctly at least some if not most of the time, but the code responsible for "contracting" the number of "database records" at that same level remains buggy.
The only support I can think to offer right now is: whatever "recursive work" was performed to "flatten" the "device list" and "expand" the "running device list table" (device_list list) in the "EAH database" needs to be undone to "unflatten" and "shrink" the "running device list table". So the higher dimensional list length (the not-recursively-flattened device_list list length) is used (again) when the "master device_list index" (db record) position gets recalculated.
Helpful?
Wow, thank you very much for the extensive reply! :-)
You/We should be able to go back and review the original "EAH class" commits to be sure, but as I recall, the goal of the "encoder assignment history" class was simply to move that code out of the "EncoderController" class where it was "visually / cognitively polluting" our understanding of how the LCD text was getting updated. In other words, all the code in "EAH" should be functionally equivalent to the same code in "EncoderController" before the code moved to "EAH". Which means, IMHO, this bug is "architectural" versus being the result of a typo or something easy to fix.
That's my understanding as well. And I've been reviewing EAH a lot in the last couple of days. More of that further down below.
Again, all I tried to do was move the existing "assignment history" functionality out of "EncoderController" where it obscured "encoder control" functionality into it's own class where it couldn't obscure any other code. The "master devices" are set up how and where they are because that's how the original code did it. As I recall, the "EAH database" allows for 128 "top level tracks" in a song arranged in the order "regular tracks", then "return tracks", then "master track". The reason the "master track" setup occurs later is because every "regular track" and "return track" has to be set up properly first so the "master track index" ("last return track index + 1") will be known.
I understand. The strange is though, that for all other tracks we have a def build_setup_database(self):
in EC, and only for this
mt_idx = self.__master_track_index
assert mt_idx == self.t_count # the master track index == the number of tracks and returns
devices_on_mstr_track = self.get_device_list(self.song().master_track.devices)
I had to additionally implement a get_device_list
in EAH. And that seemed strange to me.
What I mean is all of the lists that comprise the "EAH database" are fixed at length 128, and the database itself has no concept of "device racks". What I mean is, at whatever level in the "EAH database" there is a "db record" representing the index of the "selected device" in the "selected track's device chain". But when "device racks" are involved, that "index" needs to recursively account for all nested devices if any exist.
All the clues you've outlined lead me to SWAG whatever changes you made are logically correct in the forward direction, but some logic remains missing in the "cleanup direction". Thinking about what happens to the "master track index" when grouped "regular tracks" are opened/closed may be instructive in the devices and/or racks added/removed and/or opened/closed cases.
Conceptually, the evidence you present seems to describe a problem that seems very similar to correctly showing/hiding grouped tracks. At the (top) "tracks" level (in the EAH db) , the "master track index" changes by the size of the "track group" every time the show/hide grouped tracks view changes.
That's an interesting idea, BUT while I thought about implementing deviceracks like grouptracks (expandable etc), I didn't do it, specifically because I though it would fuck up the indexes/indices and that the way the script is built, couldn't cope with that way. That's why I implemented it differently. The racks should be completely transparent with the get_device_list
dunction. Basically it just finds 3 devices.
MackieC4 device_changestate: track <A-Reverb | Prometheus> tidx <0> type <1>
EC: processing device changestate of type <1> on track <A-Reverb | Prometheus> at index 0
EAH: track device list size <4> BEFORE device update
idx <0> before <Reverb>
idx <1> before <Prometheus>
idx <2> before <_prometheus (stereo)>
idx <3> before <prometheus>
passed idx <1> and device <Prometheus>
EAH no devices currently on track <False>
EAH device_added_deleted_or_changed: add event <True>, delete event <False>, change event <False>
found event index <1> and device <Prometheus>
for 'add' device event handling
not updating current track device bank from <0>
updated __chosen_plugin is now Prometheus
EC device_added_deleted_or_changed device list size <4> AFTER device_added_deleted_or_changed update
EC device_added_deleted_or_changed: idx <0> after <Reverb>
EC device_added_deleted_or_changed: idx <1> after <Prometheus>
EC device_added_deleted_or_changed: idx <2> after <_prometheus (stereo)>
EC device_added_deleted_or_changed: idx <3> after <prometheus>
Question is, if the "sent by EC" is actually updating stuff in EAH by 3, and if removing a rackdevice (=> decrease by 3) can be handled by EAH, or if EAH always solely expects increases/decreases by 1.
In EAH def device_added_deleted_or_changed(self, all_devices, selected_device, selected_device_idx):
all_devices
is received by EC and does include all devices.
I already tried:
def device_added_deleted_or_changed(self, track, tid, type):
in line 287-298 (new commit right now), because I thought the updated_idx
might not get properly updated. Doesn't fix itOK, adding ONE rack device
MackieC4 device_changestate: track <1-Audio> tidx <0> type <0>
EC: processing device changestate of type <0> on track <1-Audio> at index 0
EAH: track device list size <3> BEFORE device update
idx <0> before <Prometheus>
idx <1> before <_prometheus (stereo)>
idx <2> before <prometheus>
passed idx <0> and device <Prometheus>
EAH no devices currently on track <False>
EAH device_added_deleted_or_changed: add event <True>, delete event <False>, change event <False>
found event index <0> and device <Prometheus>
EAH/device_added_deleted_or_changed/if device_was_added:
not updating current track device bank from <0>
updated __chosen_plugin is now Prometheus
EC device_added_deleted_or_changed device list size <3> AFTER device_added_deleted_or_changed update
EC device_added_deleted_or_changed: idx <0> after <Prometheus>
EC device_added_deleted_or_changed: idx <1> after <_prometheus (stereo)>
EC device_added_deleted_or_changed: idx <2> after <prometheus>
removing/deleting the same device:
MackieC4 device_changestate: track <1-Audio> tidx <0> type <0>
EC: processing device changestate of type <0> on track <1-Audio> at index 0
EC device_added_deleted_or_changed device list size <0> AFTER device_added_deleted_or_changed update
Do I see this correct, that device deletion doesn't reach EAH?
But, deleting only one rackdevice otherwise works, and only fucks up if there are 2 or more devices on a track (but the same for normal vs rackdevices). Only the assert error is happening purely for rackdevices
UPDATE2: Deleting the second rackdevice:
MackieC4 device_changestate: track <1-Audio> tidx <0> type <0>
EC: processing device changestate of type <0> on track <1-Audio> at index 0
EAH: track device list size <3> BEFORE device update
idx <0> before <Rhea>
idx <1> before <_rhea>
idx <2> before <Rhea>
EAH/device_added_deleted_or_changed passed idx <0> and device <Rhea>
EAH no devices currently on track <False>
EAH device_added_deleted_or_changed: add event <False>, delete event <True>, change event <False>
EAH/device_added_deleted_or_changed found event index <0> and device <Rhea>
EAH/device_added_deleted_or_changed/device_was_removed: for 'delete' device event handling
So EAH gets updated when deleting the second device, but not when deleting the only device??? wth?
went into a short rabid whole re lambda expressions, but decided it's not worth it. New commit, more debugging statement, more questions π€ͺπ€ͺπ€―
EAH gets updated when deleting the second device, but not when deleting the only device???
That's actually a strong clue. That behavior sounds similar to another problem we've encountered with devices. The problem only manifested when you deleted/removed the "last device" in a track's device chain. I suspect it would be rewarding to investigate further (add more logging to see) why EAH isn't getting updated when deleting the only device. No update would mean the "selected device index" (for lack of a better term here) value is now bad, corrupt, garbage. Any further actions that depend on an accurate "selected device index" will only produce more garbage (that may or may not crash the script right away).
It occurs to me just now that the legacy script may also be struggling against another "database design" issue here, namely that the value 0 doesn't always have a distinct meaning. Simultaneously, IIRC, a zero value can mean both "no devices present on this track (EAH db level)" and "the selected (only) device on this track (EAH db level) is at index 0". (This same problem when the "selected device" is the last device in the chain because the "only" device in the chain is by definition also simultaneously both the first and last device in its chain.)
I don't remember the "EAH db" code managing a value like -1
to represent "no devices present". Isn't there also a "number_of_devices" count stored in the "EAH db"? Then the code could (or should be able to) test if a "selected device index" value of 0 is going to be valid or would result in an "index out of bounds" error (because an "empty list" doesn't have an "index 0", so _get_at(0)
method would throw an error)
or if EAH always solely expects increases/decreases by 1.
In general, I would bet this is true. I think the only way to add or remove more than one device at a time in Live is by adding or removing a rack. I would also bet the "EAH database" is not designed to handle adding, deleting, or changing more than one device at a time (is not designed to handle racks). This would be true if code like the device_added_deleted_or_changed
method (or code calling it) does not handle a list of devices added, deleted, or changed; but only handles "one device" per (add, delete, or change) "event". I bet only one device can ever "change" at a time, but such a change could be "a rack for a device" exchange, so the "more than one device" point remains as valid for "device changed" events as it is for "device added" and "device deleted" events.
With that said, I think the legacy script, specifically the legacy "EAH db", could be made to properly handle "device (and Instrument/Drum?) racks". Whether or not such a fix would be worth the effort to implement is another question entirely.
This is definitely the kind of problem that simply doesn't exist in more modern "framework based" scripts. Plenty of other problems remain to be solved associated with successfully writing custom "framework scripts", but this whole class of "script memory" problems just doesn't exist.
Have you tested my "minimal change" theory yet in any of your other "framework exploration" scripts? (override _install_mapping
method to add + 32
to the encoder feedback mapping identifier value) You should be able to "copy" any script you choose and make only this one minimal change so the encoders will work "properly" in your customized C4 version of that script. (the feedback will look like garbage until you also implement a custom C4EncoderElement class, but plain EncoderElement objects should just work "properly" after overriding the _install_mapping
method)
That's actually a strong clue. That behavior sounds similar to another problem we've encountered with devices. The problem only manifested when you deleted/removed the "last device" in a track's device chain. I suspect it would be rewarding to investigate further (add more logging to see) why EAH isn't getting updated when deleting the only device. No update would mean the "selected device index" (for lack of a better term here) value is now bad, corrupt, garbage. Any further actions that depend on an accurate "selected device index" will only produce more garbage (that may or may not crash the script right away).
I did add more logging, also comments in code, but so you don't need to check it there, I'll post it here: EC/def device_added_deleted_or_changed: first we have
selected_device_idx = next((x for x in range(len(extended_device_list))
if extended_device_list[x] == self.selected_track.view.selected_device), -1)
extended_device_list = self.get_device_list(self.selected_track.devices)
updated_idx = self.__eah.device_added_deleted_or_changed(extended_device_list, self.selected_track.view.selected_device, selected_device_idx)
then line 285:
if len(extended_device_list) > updated_idx > -1: # <--- THIS doesn't get called when deleting second (or third) rack device
BUT: the strange thing is, deleting ONE rack works (when EAH IS NOT called), only deleting the second (when EAH IS called) actually fails (as in doesn't update device row)
And tbh I still don't understand the logic behind the if len()... part. Mainly the -1 part. That's also why I tried indenting lines 285-397, because the initial value set in line 265 is updated_idx = -1
. Could you explain that to me? Also if that lines directly above are more indented that the if part, would that then take the initial of -1??
It occurs to me just now that the legacy script may also be struggling against another "database design" issue here, namely that the value 0 doesn't always have a distinct meaning. Simultaneously, IIRC, a zero value can mean both "no devices present on this track (EAH db level)" and "the selected (only) device on this track (EAH db level) is at index 0". (This same problem when the "selected device" is the last device in the chain because the "only" device in the chain is by definition also simultaneously both the first and last device in its chain.)
ok, more info:
if len(extended_device_list) > updated_idx > -3
doesn't change a thing, for the deletion of the second rack device, but deleting the last one throws an index error.I don't remember the "EAH db" code managing a value like
-1
to represent "no devices present". Isn't there also a "number_of_devices" count stored in the "EAH db"? Then the code could (or should be able to) test if a "selected device index" value of 0 is going to be valid or would result in an "index out of bounds" error (because an "empty list" doesn't have an "index 0", so_get_at(0)
method would throw an error)
EAH (should) get sent the values by EC in updated_idx = self.__eah.device_added_deleted_or_changed(extended_device_list, self.selected_track.view.selected_device, selected_device_idx)
doesn't it? What's strange to me, is that in the log (see above from yesterday) the EAH entries come first, then the EC entries, but with no EAH entries following?
or if EAH always solely expects increases/decreases by 1.
In general, I would bet this is true. I think the only way to add or remove more than one device at a time in Live is by adding or removing a rack. I would also bet the "EAH database" is not designed to handle adding, deleting, or changing more than one device at a time (is not designed to handle racks). This would be true if code like the
device_added_deleted_or_changed
method (or code calling it) does not handle a list of devices added, deleted, or changed; but only handles "one device" per (add, delete, or change) "event". I bet only one device can ever "change" at a time, but such a change could be "a rack for a device" exchange, so the "more than one device" point remains as valid for "device changed" events as it is for "device added" and "device deleted" events.
I am not so sure about thatΒ΄, if you look at the log entries from yesterday ("OK, adding ONE rack device"), it clearly shows that EAH adds 3 devices. Also when adding an additional rackdevice. The "adding" part works fine, it's just the deletion part which doesn't (besides the undo/redo problem, but this happens for ALL devices, BUT NOT in other scripts (MCU))
With that said, I think the legacy script, specifically the legacy "EAH db", could be made to properly handle "device (and Instrument/Drum?) racks". Whether or not such a fix would be worth the effort to implement is another question entirely.
As you have a pretty good grasp of German, here's a nice saying for you "vergebene LiebesmΓΌh", which roughly translates to "unreciprocated and lost courtship", meaning "doing shit which might not be worth it". And this might be the case here :-). But first of all, I LIKE what the (legacy) script can do currently A LOT. It is of tremendous value in my studio work. And secondly going so deep on this, actually teaches/taught me Python a lot more π€ͺπͺ. So while I definitely will have a look at the shiny new magic you've done (and been having lots of ideas about Step Sequencers etc), for now I wanna see how far I can take this old ship. I also completely understand what you're saying, it reminds me a bit of what I was proposing last year ππ
another thing: EC sends the tracks to EAH with self.__eah.build_setup_database(self.song())
in line 153.
Then EAH goes ahead and does devices_on_track = tracks_in_song[t_idx].devices
in line 105. and I am very unsure, if that can do rackdevices. Can/Should I also send the devices to EAH? And how would I do that?
I finally pulled your recent commits down to local and checked out 2022_dev for a closer look. I don't know how we're going to get to the bottom of the story, but I'm confident we can do it. I'll start with a little "groundwork".
<--- THIS doesn't get called when deleting second (or third) rack
Be careful about the conclusions you draw when debugging. The line 285 comment quoted here is inaccurate, and could lead to deep "vergebene LiebesmΓΌh" rabbit holes. (Correct use of the idiom?) What I think the comment actually means is "This block-of-indented-code doesn't get entered when...". The if
condition check is always "called". Is "doesn't get called" the direction from which you've been approaching this research? If so, it doesn't seem like a fruitful approach perspective to me.
Instead of copy/pasting code into the discussion, I created a new 2022_dev_debugging branch and updated the device_added_deleted_or_changed
method. Please don't get upset that I "moved your cheese" or whatever rearranging things. Rearranging stuff often helps me understand what's going on. (PyCharm warns me about lines longer than 120 characters too, so I try avoid that warning if possible)
The main logic I think I updated was adding an explicit check if updated_idx == -1:
at the top of those condition checks starting at line 301. Because if the updated_idx
value remains -1
AFTER selected_track
and selected_device
are validated (or not), that's a condition the code can handle and forget about. Then I didn't change the next two condition checks, but the second one elif len(extended_device_list) > 0:
(and the final else?) might now be redundant checks.
Something I didn't update, but could probably be safely implemented is to only reorder parameters and rebuild the midi map if when something actually changed. (For example when the Track changes and neither old nor new Track have a device loaded, then "nothing changes" about the "selected device" on the new Track, right?)
if something_changed:
self.__reorder_parameters()
self.__reassign_encoder_parameters(for_display_only=False)
self.request_rebuild_midi_map()
I haven't yet tried running the changes on this branch I just pushed. I don't have enough time at my disposal right now, later though... Mostly what I was trying to do was add logging to account for every execution path through the method.
And tbh I still don't understand the logic behind the if len()... part. Mainly the -1 part. That's also why I tried indenting lines 285-397, because the initial value set in line 265 is updated_idx = -1. Could you explain that to me? Also if that lines directly above are more indented that the if part, would that then take the initial of -1??
Did my changes help you follow the "if len() part" logic? Did my added "else condition comments" help you understand the logic flow? Does the updated logging make more sense and/or reveal any new details that weren't being logged before?
Since 0 is a valid "index value" for any "list" that isn't empty, a zero index value can't be used to "also" mean "the list is empty" or "nothing matches". "Traditionally" (in other programming languages) negative numbers are NOT valid indexes into lists (enumerable objects), so the "traditional" way to pass an "index value" around that means "no such index found" (or whatever) is to use a negative number like -1
. and then test if idx > -1:
before using the index to get the enumerated item. In Python though, negative numbers are perfectly valid indexes that count from the back of the enumerable. var = list[-1]
assigns the last item in the list to var
, and -2 would assign the next-to-last item to var
. So, Python programmers need to be extra careful when using this "traditional" style of indicating "no such index found".
Does that help explain the code logic better? What part(s) still remain confusing?
Edit: Pushed up another commit on the debugging branch where I also updated the logging and a little logic in the EAH "device added, deleted, or changed" method. Does the updated logging output make any more sense to you?
thanks! Testing now. (will update this post)
Single '}' encountered in format string in EAH 295 after the {0} (fixed this, do you want me to push to your branch?)
when deleting second rackdevice, I get an Attribute error with 'NoneType' object has no attribute 'format'
in EAH 362. called by EC 395 selected_device, selected_device_idx)
. So is the selected_device or the selected_device_idx = None?
Which reminded me of https://github.com/xot/ElectraOne/blob/main/DOCUMENTATION.md#device-appointment that was another rabbit whole I got myself into in the last couple of days. Trying to understand the difference between appointed and selected device (in the Python API), what do we use and if, what Henk writes in the documentation, might be that exact problem ("The problem is that self.song().appointed_device is shared by all remote controllers and their scripts, but that Live does not itself handle the appointment of the currently selected device. The remote script needs to do it.").
Preceding the attribute error, is
C4/device_changestate: track <1-Audio> tidx <0> type <0>
EC/device_added_deleted_or_changed: processing device changestate of type <0> on track <1-Audio> at index 0
EC/device_added_deleted_or_changed: if liveobj_valid(self.selected_track.view.selected_device): <Device.Device object at 0x000000004F635BF8>
EAH/device_added_deleted_or_changed: input device list len<3>
EAH/device_added_deleted_or_changed: device in input device list at index<0> is a valid Live object named <Rhea>
EAH/device_added_deleted_or_changed: device in input device list at index<1> is a valid Live object named <_rhea>
EAH/device_added_deleted_or_changed: device in input device list at index<2> is a valid Live object named <Rhea>
EAH/device_added_deleted_or_changed: input selected_device is a valid Live object named<Rhea>
EAH/device_added_deleted_or_changed: input selected_device_idx<2> points to a forward index
EAH/device_added_deleted_or_changed: input selected_device_idx<2> and input device list len<3> agree that at least one device currently populates the device chain for this track
EAH/device_added_deleted_or_changed: add event <False>, delete event <True>, change event <False>
EAH/device_added_deleted_or_changed: matched input selected_device<Rhea> with device<Rhea> at index<2> of input device list
{0}device_was_removed: for 'delete' device event handling
This is when selecting by mouse one of the devices within the rack or the rack itself. So change event should be TRUE, but it isn't. Could also be that the index is too fucked up at that point to deal with changes π I also get the error from above if using 2 normal devices and deleting on of them, that didn't happen before?? π€π€
The line 285 comment quoted here is inaccurate, and could lead to deep "vergebene LiebesmΓΌh" rabbit holes. (Correct use of the idiom?)
ππππ
What I think the comment actually means is "This block-of-indented-code doesn't get entered when...".
You're correct, my dev nomenclature is not good ππ
I created a new 2022_dev_debugging branch and updated the
device_added_deleted_or_changed
method. Please don't get upset that I "moved your cheese" or whatever rearranging things. Rearranging stuff often helps me understand what's going on. (PyCharm warns me about lines longer than 120 characters too, so I try avoid that warning if possible)
Not upset, but grateful and appreciative for the help! Your logging foo is way superior to mine and very helpful! W re to line length, I'm the opposite. pycharm doesn't complain to me, only shows the subtle hint, which I ignore, because putting nearly everything on one line makes it easier for me to grasp what's going on, and also reduces scrolling ;-)
Something I didn't update, but could probably be safely implemented is to only reorder parameters and rebuild the midi map if when something actually changed.
Not sure, because changing tracks or switching assignments actually works great and also cleans up the indexes (even if they were wrong before), so maybe we need to do MORE request_rebuild_midi_map
instead of less?
Mostly what I was trying to do was add logging to account for every execution path through the method.
π Thank you, and great idea with the log_id and log_msg, shortens stuff significantly!
Did my changes help you follow the "if len() part" logic? Did my added "else condition comments" help you understand the logic flow? Does the updated logging make more sense and/or reveal any new details that weren't being logged before?
9. I am aware of the -1 enumerable thing in Python, my question was more about the math/logic. Good to know the reasoning behind using -1 stuff in general! βΊ
No Problem, glad to help. Especially because you're spamming "Thank You"s. :)
Sure, no problem. That specific }
problem is something I would have found too as soon as I let Live compile the script again. The only time I can imagine I might mind something like that would be if it appeared completely out of the blue. Like I'm 50 commits down a branch you've been ignoring then suddenly without warning you submit a little "typo fix" commit like that. I can imagine feeling "WTF?", but barely. Mostly, even then, I imagine feeling "welcome, what made you finally check in?"
Attribute error with 'NoneType' object has no attribute 'format' in EAH 362
This issue might be related this log line {0}device_was_removed: for 'delete' device event handling
Because format
is that "String interpolation method" we've been using for log messages, the error would mean the code is trying to call the format
method on a "None type" object instead of a "String type" object. Since the {0}
placeholder "belongs with" the "first format parameter", {0}
should be "replaced by" (interpolated into) the "first format parameter" when the string is "written out" to the log. Since the {0}
appears in the log, there is some associated problem with the associated format
call.
I might have guessed "unrequited love" as the translation depending on how much surrounding context I correctly decoded. But the idiom is so much more general and less creepy. Continuing to pursue an unrequited love because "vergebene LiebesmΓΌh" would be creepy, right? Crossing toward harassment these days?
<skip 4>
Also, vertical scrolling is way less of a cognitive burden on me than horizontal scrolling. The mouse wheel works great by default. I don't mind line lengths that go beyond the right edge of the screen until I need to "study" the code. Then, it's really annoying to me when I have to keep "manually" mouse dragging the horizontal scroll bar back and forth to see everything. (in addition to scrolling up and down) (My normal PyCharm "workspace window layout" on this 32 "inch class" monitor could accommodate line lengths of maybe 175 or 180 characters without horizontal scrolling, and but most window layouts on 24 and 27 "inch class" monitors can still accommodate line lengths of 120 characters without horizontal scrolling. If anyone using a small monitor ever "joins" this project, they might request adopting some specific "shorter" line length coding standard. Until then, I only change what I'm editing when I'm editing (or researching). Generally, even though I still don't understand a lot of those gray squiggly lines PyCharm throws out, I've seen enough of them point directly at eventual "Live compile time" or "Live run time" issues that I try to clear them up when I see them. I only "ignore" the ones I understand and agree to live with. Then I usually explicitly tell PyCharm "no inspection" here and temporarily turn the "next-line inspector" off so I don't have to ignore what I don't see.
I understand your point about vertical scrolling too though. Because I think super short line length standards suck even more than super long or no standards. The "Java API docs" are the prime example coming to mind. Most of that code seems to be written for display on 40 or 50 character lines. Why, why, why? If a method call with all parameters would all fit on one line on an 80 or 100 character line, why do they break the method call line into a line for every parameter. Some of that is just legacy inertia. Old school consoles, monitors, and paper prints are only so wide, there's no such thing as "off screen to the right" in those environments. Nobody want to take the time to "reformat" all that API code so it looks better (and is easier to read) on modern systems. (Also, my inner skeptical-of-commercial-motives cynic says that kind of narrow source code formatting fits very well on a narrow web page with wide or very wide "advertising borders".)
My "logging Fu" (meme reference to the 1970s TV show "Kung Fu") has been honed by years of reading my own log messages and seeing both perspectives clearly. What I mean is when you are writing log messages, you are by definition writing code and "code writing time" is generally the perspective from which the log messages are written. Then, when you are reading log messages, (by definition?) you are "bug hunting" or "problem solving" where your "reading perspective" is NOT "code writing time" it is "run time". (Often months or years after the code and logging messages were written so all memory from "code writing time" has basically vanished and all you have to work with is what the logs are now "telling you".)
So now, when I'm writing log messages, I try to imagine myself reading those log messages later at run time. The messages still need to relate to the code around the log message, but generally a log message should use language from the code's "run time domain" versus language from the code's "compile time domain". If a logged message only "actually" makes sense to me after I go back and look at the code where it came from, the language of that message usually needs to change to "make more sense". The logs should "read like" a book more or less, that's the target at which I'm generally aiming anyway. An application's "run time execution pointer" follows a path through the app's source code that tells a story and "log messages" can often be the only way to follow the story as and after the story runs (the book is written). (Certainly true for Control Surface Python scripting where we don't have a proper "debugger" to run our code in. Since we can't set "breakpoints" to forensically step through and examine the state of a running Control Surface script, all we have to work with is the story the logs tell.)
-1
instead of 0
from the EAH (add, delete, change device ADCD) method more often or that the real problem is somewhere else like an "out of date" index value sometimes leading to a GIGO problem in ADCD. That suspicion might align with your suspicion about hitting rebuild_midi_map()
more often too. I wasn't thinking about how often devices "change" from a Control Surface script's perspective before yesterday. Every time the selected track changes, the appointed device could change too. The device chain on the old Track gets "unmapped" and device chain on the new Track gets "mapped" in the script. If the old "selected Track" had 4 devices in its device chain and the new "selected track" has 0 devices, then yes like you said, the ADCD method(s) will handle "deleting" all 4 devices in one pass.
I'm not sure about my interpretation, but I currently think of the difference between "selected device" and "appointed device" in terms of "locking" a script to a specific device. While a script is "locked" to some device, its "appointed device" reference stays locked on that device. But otherwise, I think an "unlocked script's appointed device" follows Live's "currently selected device" around like an alias. (Or should?)
I'll try running the legacy debugging script here later today. But I'm not sure exactly how you are triggering the issues being reported.
It might be beneficial to agree on some common testing definitions going forward, things like using a common "song session" structure to test with. (Like the standard testing song has 4 tracks and every Track has 4 devices in the device chain (any of the 4 could be another chain). Then we could maybe abolish ambiguous phrases like "the second device on the last track fails when..." and use more concrete references to our standard testing structure like tracks[3].devices[1] fails when...
Fully agree on testing procedure. Quickly typing from phone, my test song has exactly one audio track + master, nothing else. And then I always start fresh by pulling either normal devices (usually bx_meter) or rack devices (with 2 devices enclosed in a rack device) onto the audio track
I'll try running the legacy debugging script here later today. But I'm not sure exactly how you are triggering the issues being reported.
basically this https://github.com/markusschloesser/MackieC4_P3/issues/48#issuecomment-1368508088 but let's focus on rackdevices first and maybe the undo/redo problem will be solved by this as well
Testing Notes: Default session contains 5 tracks + 2 return tracks (VST, Inst, sample, audio input, midi) + (reverb, delay) The only "Devices" on the VST and Inst Tracks are the VST or Live (midi) instruments.
Track 2 is selected when the session loads where the only device is "Wavetable"
When the session first loads, a "track change" event is "immediately" handled by the script because track 1 is the script's default selection.
(MackieC4) t_current idx <0> t_count <0> BEFORE track change
(MackieC4) t_current idx <1> t_count <0> AFTER track change
(MackieC4) EC track changed, selected device index = -1
This seems odd, or could be. Shouldn't t_count > 0
be True both before and after track change? And also, the "new" selected track has a Wavetable device on it, but it's not the selected device? It might not be a lingering problem if at all because later the logged track count is more correct (but still might be "off")
(MackieC4) t_current idx <1> t_count <0> BEFORE setup_db
(MackieC4) nbr tracks in song 5
(MackieC4) t_current idx <1> t_count <7> AFTER setup_db
(MackieC4) t_count after setup <7>
(MackieC4) main_script().track_count after setup <7>
What might be an issue is that 5 is the correct number of normal tracks and 7 is the correct number of normal + return tracks in the session. Need to validate that this logging is correct for what we should expect in this case. Should we expect return tracks included in the track count like this? always?
ACTION: Delete Wavetable device on Track 2 Logged Result: OK
ACTION: Add Wavetable device back again (Drag&Drop from browser) Logged Result: OK
ACTION: Add Rack device to chain (Warped & Rising) Logged Result: OK
ACTION: Delete Rack device from chain Logged result:
RemoteScriptError: self.main_script().log_message("{0}device_was_removed: for 'delete' device event handling").format(log_id)
RemoteScriptError: AttributeError
RemoteScriptError: :
RemoteScriptError: 'NoneType' object has no attribute 'format'
RemoteScriptError:
Specifically fixed (in several places) by calling format() on the string instead of the return from main_script().log_message()
self.main_script().log_message("{0}device_was_removed: for 'delete' device event handling").format(log_id)
self.main_script().log_message("{0}device_was_removed: for 'delete' device event handling".format(log_id))
After this fix to some of the logging calls, I'm not seeing any errors (or data problems?) logged after "deleting" devices or racks. Are you still seeing any issues? If so, please describe how to reproduce.
Add a second rack device and then delete it. Devices row in Chan strip mode will still show the deleted device(s), this will also fuck up the index, meaning consecutive device stuff will probably throw errors. My problem only appears when there's at least 2 rack devices are loaded and then one is deleted.
With 4 Rack devices loaded on an audio track (the "audio input" track noted above) First I reordered the rack devices and or changed selected rack
__chosen_plugin is now Eastern Brain because updated index is <1>
device count AFTER update <4>
device at index <0> is <Stun Echo>
device at index <1> is<Eastern Brain>
device at index <2> is<Neptune Rising>
device at index <3> is<Elephant Smile>
__chosen_plugin is now Eastern Brain because updated index is <3>
device count AFTER update <4>
device at index <0> is <Stun Echo>
device at index <1> is<Neptune Rising>
device at index <2> is<Elephant Smile>
device at index <3> is<Eastern Brain>
__chosen_plugin is now Stun Echo because updated index is <0>
device count AFTER update <4>
device at index <0> is <Stun Echo>
device at index <1> is<Neptune Rising>
device at index <2> is<Elephant Smile>
device at index <3> is<Eastern Brain>
__chosen_plugin is now Stun Echo because updated index is <3>
device count AFTER update <4>
device at index <0> is <Neptune Rising>
device at index <1> is<Elephant Smile>
device at index <2> is<Eastern Brain>
device at index <3> is<Stun Echo>
__chosen_plugin is now Elephant Smile because updated index is <1>
device count AFTER update <4>
device at index <0> is <Neptune Rising>
device at index <1> is<Elephant Smile>
device at index <2> is<Eastern Brain>
device at index <3> is<Stun Echo>
Then I started deleting. The deleted rack wasn't always the "last rack in chain" and the count goes 3, 3, 2, 2, 1, 0 because in order to delete, I first selected the rack to delete if it wasn't already selected.
__chosen_plugin is now Eastern Brain because updated index is <1>
device count AFTER update <3>
device at index <0> is <Neptune Rising>
device at index <1> is<Eastern Brain>
device at index <2> is<Stun Echo>
__chosen_plugin is now Stun Echo because updated index is <2>
device count AFTER update <3>
device at index <0> is <Neptune Rising>
device at index <1> is<Eastern Brain>
device at index <2> is<Stun Echo>
__chosen_plugin is now Eastern Brain because updated index is <1>
device count AFTER update <2>
device at index <0> is <Neptune Rising>
device at index <1> is<Eastern Brain>
__chosen_plugin is now Neptune Rising because updated index is <0>
device count AFTER update <2>
device at index <0> is <Neptune Rising>
device at index <1> is<Eastern Brain>
__chosen_plugin is now Eastern Brain because updated index is <0>
device count AFTER update <1>
device at index <0> is <Eastern Brain>
__chosen_plugin is now None because no EAH updated index
device count AFTER update <0>
new_device_count_track was NOT > 0, NOT enumerating devices for log
Either the problem is fixed already or I'm still not doing something you are.
I'm assuming the answer will depend on your results after you return from your weekend trip.
π But does it actually clean up the devices row on the C4 itself?
Ha, I wasn't paying attention to the C4 displays. I was watching logs.
But yes, I double checked and in channel strip mode both the selected device name in the top LCD and the device chain member names (each rack name) in the second display appear to update correctly when racks are deleted.
Wow that's good to hear! Will check out tomorrow evening when I'm back! FYI the top was always correct, but that's because it does things differently in on_update_display_timer. For the second row, I followed the trail all the way from on_update_display_timer to adcd in ec and eah. Now very curious! Does undoing redoing adding device or deleting device work as well (ON C4)?
I'm not sure how exhaustively I tested all the scenarios. I did test add-once/delete/add-again, but I'm not sure I ran it with racks, and, I was looking at logs more than the displays until I went back. Now, I've got other stuff planned today so the studio is shut down.
All the logic changed before you last tested. The two issues I fixed today were both typos (and copy/pasted) related to the updated logging. You had already found and fixed the "extra }
in formatted string" issue.
ok, back home and testing. Unfortunately I still see the errors:
C4/device_changestate: track <1-Audio> tidx <0> type <0>
EC/device_added_deleted_or_changed: processing device change-state of Track listener type <normal> on track <1-Audio> at index 0
EC/device_added_deleted_or_changed: if liveobj_valid(self.selected_track.view.selected_device): Hyperion
EAH/device_added_deleted_or_changed: input device list len<3>
EAH/device_added_deleted_or_changed: device in input device list at index<0> is a valid Live object named <Hyperion>
EAH/device_added_deleted_or_changed: device in input device list at index<1> is a valid Live object named <_hyperion (stereo)>
EAH/device_added_deleted_or_changed: device in input device list at index<2> is a valid Live object named <Hyperion>
EAH/device_added_deleted_or_changed: input selected_device is a valid Live object named<Hyperion>
EAH/device_added_deleted_or_changed: input selected_device_idx<0> points to a forward index
EAH/device_added_deleted_or_changed: input selected_device_idx<0> and input device list len<3> agree that at least one device currently populates the device chain for this track
EAH/device_added_deleted_or_changed: add event <False>, delete event <True>, change event <False>
EAH/device_added_deleted_or_changed: matched input selected_device<Hyperion> with device<Hyperion> at index<0> of input device list
EAH/device_added_deleted_or_changed: device_was_removed: for 'delete' device event handling
EAH/device_added_deleted_or_changed: device_was_removed: deleted_device_index<0> device_count_track<6>
EAH/device_added_deleted_or_changed: param_count_track: replacing<1, 0> with<0, 18>
EAH/device_added_deleted_or_changed: param_bank_count_track: replacing<1, 0> with<0, 1>
EAH/device_added_deleted_or_changed: param_bank_current_track: replacing<1, 0> with<0, 0>
EAH/device_added_deleted_or_changed: param_count_track: replacing<2, 0> with<1, 18>
EAH/device_added_deleted_or_changed: param_bank_count_track: replacing<2, 0> with<1, 1>
EAH/device_added_deleted_or_changed: param_bank_current_track: replacing<2, 0> with<1, 0>
EAH/device_added_deleted_or_changed: param_count_track: replacing<3, 18> with<2, 18>
EAH/device_added_deleted_or_changed: param_bank_count_track: replacing<3, 1> with<2, 1>
EAH/device_added_deleted_or_changed: param_bank_current_track: replacing<3, 0> with<2, 0>
EAH/device_added_deleted_or_changed: param_count_track: replacing<4, 0> with<3, 18>
EAH/device_added_deleted_or_changed: param_bank_count_track: replacing<4, 0> with<3, 1>
EAH/device_added_deleted_or_changed: param_bank_current_track: replacing<4, 0> with<3, 0>
EAH/device_added_deleted_or_changed: param_count_track: replacing<5, 0> with<4, 18>
EAH/device_added_deleted_or_changed: param_bank_count_track: replacing<5, 0> with<4, 1>
EAH/device_added_deleted_or_changed: param_bank_current_track: replacing<5, 0> with<4, 0>
File "C:\ProgramData\Ableton\Live 11.1\Resources\MIDI Remote Scripts\MackieC4SissyTest\MackieC4.py", line 834, in
Devices row on Mackie C4 does NOT update.
Ableton Live Suite 11.2.7
Tested with my own Racks, containing 1 VST3 Plugin and 1 external audio effect (Live inbuilt) and your latest commit
BUT!
I also tested with Live native devices and there is a difference when it comes to errors!
with 2 Live native devices (Instruments/Instrument Rack/Ambient&Evolving/Abrasive.adg and Audio Effects/Utilities/Audio Effect Rack/Amp Simulation/Basic Heavy Guitar Amp.adg), I do NOT get the assertion error from above BUT the Mackie C4 Devices row in ChanStrip Mode does not update (does not remove the deleted device).
Logs:
C4/device_changestate: track <1-Abrasive> tidx <0> type <0>
EC/device_added_deleted_or_changed: processing device change-state of Track listener type
The reason for not getting the assertion error is:
The second rackdevice (Basic Heavy Guitar Amp.adg) initially only shows ONE device. That's because that specific device chain, while existing, is hidden (only the rackdevice is shown) and the get_device_list function respects that. If I toggle that "Show/Hide Devices", 2 things happen:
1. The Devices row still shows only one Device (changing tracks and coming back properly updates the C4 Devices row)
2. When I delete that now expanded rackdevice, I get the same assertion error as above.
3. When I undo the loading of the collapsed Basic Heavy Guitar Amp.adg, it works!
4. When I then redo the loading, the Basic Heavy Guitar Amp.adg loads EXPANDED???
What are you testing with? And why does your Devices row update properly?
I will now parse the logs and try to understand them.
I was basically testing with native Live Instruments, Audio Effects, and Effect Racks, either on a "naked" audio track or an Instrument track (Wavetable) [in a session with 5 normal + 2 return tracks]. Definitely not a custom rack I created myself, and not with "expanded" (or previously expanded) Racks in the chain. I was able to add and then delete 4 and 5 device racks without seeing issues, but I never "opened" any racks.
I'll test again too, with this new information in mind.
I suspect the problem logged above is that the ADCD method input devices list only contains 3 elements input device list len<3>
when the EAH database shows size 6 device_count_track<6>
when the actual counts should agree like they do at 5 in the second example. I'm not convinced 5 is correct in the second example, but at least those counts agree (and "prevent" the assertion issue?).
The second example is also instructive because it reveals exactly which log line DID NOT get logged after the previous Assertion error example. In this specific case, we have the assertion error line for a smoking gun. But often one of your first debugging clues is finding that "next" log line that didn't get logged in order to finish defining the boundary around where to look for the problem.
This is key to solving the issue. (IMHO)
the get_device_list function respects [the fact] that specific device chain[s can be] hidden.
The EAH "database" needs to learn the same amount of respect.
Edit: I reproduced the Assertion error, and I think the most underlying issue is the script is not responding to "rack opened/closed (show/hide?)" events at all. (No listener?) But when a rack is on a track's device chain and open and then some other event happens (that the script does respond to), then all the rack's visible devices "pollute" the EAH database. When those "polluting items" get hidden again or the whole rack of polluting items gets deleted, the "lingering effects of the pollution" are felt in terms of the errors (either immediate errors or "next event response" errors). I'm only using the term pollution to describe the problem. If "garbage" gets taken out properly, it doesn't turn into "pollution".
I suspect we could solve the issue at the source if the script responded to "show/hide rack device chain" events exactly like it responds to ADCD events (by adding, deleting, or changing rack-chain devices to, from, or in the track's "visible" device chain).
For example, whenever a device is added, the script needs to check if the added device is a Rack and "add a Listener" to the Rack's "Show/Hide devices in chain" event (the Listener method passed to the event would be the same ADCD listener method the script already uses for the Track chain's Listener) And whenever a device is deleted, the scripts needs to check if the deleted device is a Rack and "remove the Show/Hide Listener" when appropriate.
Theoretically, this fix could lead directly toward specializing the "channel strip" mode "device row" encoder buttons so that IFF a given device connected to an encoder is a Rack device, then the encoder button acts like a "Show/Hide Rack's chained devices" toggle (staying in "channel strip" mode) instead of jumping to "device" mode (for a normal device connected to the same encoder).
I was trying to keep it simple and try to avoid the whole show/hide thing, because dynamically changing amounts of devices sounded messy. I was right π
IMHO we can still keep it simple, just by removing if device.view.is_showing_chain_devices:
from def get_device_list(self, container)
. Then, all devices, no matter if expanded or not, would always be listed.
Also, and I'd like to emphasize that, is that, when a rackdevice is not expanded, it currently is basically ONE device. That's why you didn't see any errors before. But the errors I was and am seeing, are imho not caused by expand/hidden chains, but due to the fact, that is is a rack/contains multiple devices.
I'm not convinced 5 is correct in the second example, but at least those counts agree (and "prevent" the assertion issue?).
It is correct, Abrasive.adg contains 5 devices. You should be able to test yourself, iirc it is part of the standard library.
Edit: I reproduced the Assertion error, and I think the most underlying issue is the script is not responding to "rack opened/closed (show/hide?)" events at all. (No listener?)
correct, the corresponding listener would be is_showing_chain_devices_has_listener
. The original script doesn't take that into account, because at that time, Live didn't have chains.
Theoretically, this fix could lead directly toward specializing the "channel strip" mode "device row" encoder buttons so that IFF a given device connected to an encoder is a Rack device, then the encoder button acts like a "Show/Hide Rack's chained devices" toggle (staying in "channel strip" mode) instead of jumping to "device" mode (for a normal device connected to the same encoder).
I had thought about that and decided against it, because of the complexity and also how would you then select a device? The "flat list" implementation is imho easier and has no drawbacks besides having lots of devices shown (but the banking feature for the device row works beautifully! :-) )
I suspect simply flattening the device chain as you suggest will only kick the can down the road a bit rather than actually solve the problem.
Imagine what will happen when the encoder button associated with a hidden device (but still "mapped" to an encoder in channel strip mode) triggers a switch to device mode?
I predict the script won't be able to connect (encoders) to the parameters of a hidden device, so device mode for a hidden device will be "all jacked up". But, that's why science runs experiments (why programmers run tests). It sounds like you know exactly how to implement the remedy you have in mind. Go for it. Let's see what happens.
I did not mean anything like the "Chain" and "Hide" radio buttons visible in the audio-effect-rack's "mixer view" (whatever those do)
Edit:
But the errors I was and am seeing, are imho not caused by expand/hidden chains, but due to the fact, that is is a rack/contains multiple devices.
We may just not be talking the same language here. The way I am using the word "chain" relates in this way. (IMHO) The "only" difference between a normal "audio effect" device and an audio-effect-rack "audio effect" device is that the audio-effect-rack device also "hosts" a "device chain" just like a "mixer track" device "hosts" a "device chain". What I mean by "Show/Hide" the "device chain" of an audio-effect-rack device is, for example... (Pictures would come in handy)
Edit 2: I validated the above steps are generally reproducible after I commented by dragging the "Underbelly Synth Blanket" rack to the end of a Track's device chain at step 1, an "EQ3" device to the end of the Track's device chain at step 4, and a "Tremolo" device to the spot before the "Underbelly Synth Blanket" rack in the Track's device chain at step 6. Boom!!
Also, you can make the error go away by showing the hidden devices again (double click on the "chain mixer track" again), and the error will repeat in cycles as long as you alternate between showing or hiding the rack's "mixer track device chain" and doing a "listener event" that activates the ADCD methods. All you need to do to go Boom at step 6 after "hiding" devices at step 5 is change the selected device.
I suspect simply flattening the device chain as you suggest will only kick the can down the road a bit rather than actually solve the problem.
Imagine what will happen when the encoder button associated with a hidden device (but still "mapped" to an encoder in channel strip mode) triggers a switch to device mode?
I predict the script won't be able to connect (encoders) to the parameters of a hidden device, so device mode for a hidden device will be "all jacked up". But, that's why science runs experiments (why programmers run tests). It sounds like you know exactly how to implement the remedy you have in mind. Go for it. Let's see what happens.
Au contraire! :-)
I commented out the # if device.view.is_showing_chain_devices:
and then:
Loading the above mentioned "Basic Heavy Guitar Amp" (a rackdevice with collapsed devices) onto an audio track, shows a total of 6 devices. If I then select one of the (hidden) one via vpot push, Live opens the chain, and selects the device, no errors whatsoever.
re
We may just not be talking the same language here.
I will post pictures, but I do mean racks (which can contain chains), and the button I am talking about is the "Show/Hide Devices" button. Racks are "grouped" devices, which you can also produce by yourself, just by multi-selecting 2 or more devices, right-clicking and then click "Group". Racks/Groupdevices can also be nested. See https://www.ableton.com/en/manual/instrument-drum-and-effect-racks/ 20.2
committed and pushed to your branch a second ago.
The main error (leaving the undo/redo problem aside for now), appears once you have 2 or more expanded rackdevices on a track and then delete one of them. Moving rackdevices around (changing their position with in the track) works fine
pulling and looking.
We are kind of dancing around terms though. You are saying "the main error" takes two or more expanded rack devices to trigger the problem and I am saying it only takes one expanded rack that gets hidden, shrunk, de-expanded again to trigger the problem as long as you do a "listener action" in between expanding and de-expanding. In terms of this error, I think deleting an entire second expanded rack device is functionally equivalent to just hiding the chained (nested?) devices in one expanded rack device. Either way, the next "listener event" the script goes off the rails for a while, until the events can line up again.
"My main error" :-) Also, with the "always_flattened_list" approach, "your_main_error" is imho no longer relevant/happening. Even when we fix the "my_main_error", it would additionally need the implementation of the device chain listener I mentioned earlier to enable the script to handle expand/collapse of rackdevices. But that is on top/After the "my_main_error" :-)
Lol, I just meant you named it "the main" error. Also to contrast with "my encounter" with a version of the error.
That change "fixed" the show hide chained devices issue I was seeing. "Fixed" is in quotes because it seems like a "stop the bleeding" kind of solution rather than a "real" solution. I see the "main error" still exists though. I'm going to keep playing with this to see if I can identify the pattern now.
This "main error" still seems like the same class of problem as that show/hide devices issue to me...
Edit: I'm finding it only takes one rack device plus any other device in the chain to cause the main problem now. Since "hidden" devices are always "showing" (as far as the script is concerned), the assertion error always appears when I delete a rack device that wasn't the only device in its chain. It looks like I can wipe away the problem by changing selected track and back.
Lol, I just meant you named it "the main" error. Also to contrast with "my encounter" with a version of the error.
I know ππ
Edit: I'm finding it only takes one rack device plus any other device in the chain to cause the main problem now. Since "hidden" devices are always "showing" (as far as the script is concerned), the assertion error always appears when I delete a rack device that wasn't the only device in its chain. It looks like I can wipe away the problem by changing selected track and back.
yep, I just remembered that I wrote that earlier in this epic thread π
parsing the logs:
when you say deleted_device_index
in EAH, do you mean "the device(s)" that were deleted? Or the list of devices before delete?
I agree that we sometimes have a language/nomenclature issue, that's why I am asking. This is also one of the reasons that I am struggling with EAH (beside the fact that you are way more proficient in Python)
Also parsing:
Since my "default session" has a Wavetable "Descenting Dreams" device on the selected track when it loads, all I need to do to trigger the main error is add a rack to that selected track's device chain and then immediately delete the rack just added. Then all of these log entries probably correctly reflect the "new" state after deleting the rack, until the last line:
processing device change-state of Track listener type <normal> on track <2-Descenting Dreams> at index 1
if liveobj_valid(self.selected_track.view.selected_device): Descenting Dreams
input device list len<1>
device in input device list at index<0> is a valid Live object named <Descenting Dreams>
input selected_device is a valid Live object named<Descenting Dreams>
input selected_device_idx<0> points to a forward index
input selected_device_idx<0> and input device list len<1> agree that at least one device currently populates the device
chain for this track
add event <False>, delete event <True>, change event <False>
matched input selected_device<Descenting Dreams> with device<Descenting Dreams> at index<0> of input device list
device_was_removed: for 'delete' device event handling
device_was_removed: deleted_device_index<0> device_count_track<8>
device_count_track<8>
is definitely wrong at this point. The correct value is input device list len<1>
above. And deleted_device_index<0>
is probably wrong too. The deleted rack device was at index 1 (the last device in the track's chain). At this point index 0 is the current "last device in the track's chain" (the only device, Wavetable). All the indexes beyond 0 should be getting wiped out of the list but only one device is getting dropped.
assert new_device_count_track == self.t_d_count[self.t_current]
is failing because new_device_count_track
holds a correct value of 1 but self.t_d_count[self.t_current]
holds an incorrect value like 7 (because 8 - 1 = 7)
or at least that's what I'm thinking right now...
device_count_track<8>
is definitely wrong at this point. The correct value isinput device list len<1>
above. Anddeleted_device_index<0>
is probably wrong too. The deleted rack device was at index 1 (the last device in the track's chain). At this point index 0 is the current "last device in the track's chain" (the only device, Wavetable). All the indexes beyond 0 should be getting wiped out of the list but only one device is getting dropped.
that's EXACTLY the reason why I was asking! :-) Because I saw that too.
We do send the updated extended_device_list
over from EC, so things should be up to date?
side question: Does the name of a function parameter need to be the same when it is "called" (not sure about nomenclature, the argument?)? Asking because def device_added_deleted_or_changed(self, **all_devices**, selected_device, selected_device_idx)
in EAH, but call is updated_idx = self.__eah.device_added_deleted_or_changed(**extended_device_list**, selected_device, selected_device_idx)
(all_devices vs extended_device_list)
In EAH all_devices
is the "local" name of the second input parameter of the ADCD method where the method is defined. In EC extended_device_list
is the "local" name of the variable passed as the second input parameter to the ADCD method where the method is called. The two "local" names will never conflict with each other whether matching or not.
(Edit 2 Python lesson. When you use the param_name=any_name
"named parameter" style of calling a method, the param_name
must precisely match an existing "parameter name" defined for the method or the style won't work right. I mean, on the left of the assignment operator =
you would use all_devices=
because all_devices
is the parameter name "by definition". But for normal "positional parameter" method calls the name you pass doesn't matter as long as the type of object you pass is correct.)
Now I'm thinking the problem is because when you delete any device from a track's device chain, there is also an implicit change of selected device at the same time (to the device before the deleted device or None). It doesn't matter for "single" devices precisely because there is only one of them, but when rack devices are involved and get deleted, then the selected device only "decrements by one" at the audio-effect-rack level while at the (flattened or viewable) "device chain" level the "value" of the EAH database record needs to decrement by all the devices in the rack that was deleted (by more than one)
You are correct. The "new information" from EC is correct and "up to date". The problem is EAH maybe doesn't respect what EC is telling it enough yet...
Edit: I think this hard coded decrement is our smoking gun:
if last_device_in_chain or empty_chain:
# only decrement "current device" index if deleted device wasn't the only device
if deleted_device_index > 0:
self.t_d_count[self.t_current] -= 1
else:
self.t_d_count[self.t_current] = 0
else:
# device chain is not empty and "current device" isn't the only device
self.t_d_count[self.t_current] -= 1
When a Rack device is involved, this can't always be a hard coded -1 decrement. Somehow, this code needs to know "how many" devices were actually deleted counting all (nested and visible or) flattened devices within the rack-that-was-deleted.
Just saw that as well.
amount_of_device_removed = new_device_count_track - device_count_track
?
and then
self.t_d_count[self.t_current] -= amount_of_device_removed
Update 1: tried it, doesn't fix it. And I gotta get to bed :-) Good night!
Update 2: I might have put it in the wrong place. Probably needs to go before last_device_in_chain
/ if last_device_in_chain or empty_chain:
Ha. I was wondering how late it was getting over there on the continent.
You almost had it, just backwards (in tired haste?)
This seems to have resolved the issue:
#renamed
old_device_count_track = self.t_d_count[self.t_current]
...
rack_devices_deleted = old_device_count_track - new_device_count_track if device_was_removed else 0
...
self.t_d_count[self.t_current] -= rack_devices_deleted
confirmed working!! πππ
(in tired haste?)
yes, indeed
it also solved a couple of undo/redo issues, but not all of them. Error nr 1: Test case:
Error Nr 2:
EAH/device_added_deleted_or_changed: device_was_added current track device bank updated to <3>
when it should be <2>Will investigate.
re Error Nr2:
I suspect something wrong in EAH/ def get_selected_device_bank_index(self):
and / or def get_selected_device_bank_count(self):
.
Why if len(self.t_d_bank_current) > self.t_current:
? Shouldn't it be something like if len(self.t_d_bank_current) > self.t_d_bank_count:
?
if len(self.t_d_bank_current) > self.t_current:
That logic certainly looks odd in isolation. Why would the "current track index" value have a logical relationship with the number of parameters in the current device bank? The answer is found by looking around. I mean
if len(self.t_d_bank_current) > self.t_current:
selected_device_bank_index = self.t_d_bank_current[self.t_current]
What would happen when assigning the protected return value selected_device_bank_index = self.t_d_bank_current[self.t_current]
and the "else logic" was True len(self.t_d_bank_current) <= self.t_current:
?
The self.t_d_bank_current[self.t_current]
"list item access" style is common throughout the EAH database. [self.t_current]
is the "common denominator" accessor.
Checking the two "additional" reported errors locally now, but also otherwise busy for a few hours today.
Regarding "banking", specifically "device parameter banking" but possibly banking generally across Live for Control Surface scripting, I just recently learned this:
Live has an internal concept "hard coded" that "device banks" are size 8. I have a tendency to think of the C4 encoders as a single bank of 32, or however many I want to "load" per "page" of encoders on the C4. But I think the reality is, in order to "get" 32 device parameters out of a "device", you need to fetch 4 "device banks" worth of parameters. I would bet we'll find this problem is related to some naive logic issue associated with a clash between "4 banks of 8" == 32 parameters
and "1 bank of 32" == 32 parameters
.
Edit: Do you want to close this issue and jump over to the undo/redo issue discussion?
That logic certainly looks odd in isolation. Why would the "current track index" value have a logical relationship with the number of parameters in the current device bank?
It's not the "number of parameters in the current device bank", it's the number of banks (devices/8), right? That's what your comments in init say, at least :-)
The
self.t_d_bank_current[self.t_current]
"list item access" style is common throughout the EAH database.[self.t_current]
is the "common denominator" accessor.
I know, and that makes sense to me (NOW, before I had issues understanding. Now I know it's the equivalent of self.song().view.selected_track.devices
divided by 8. Now of course it would be the devices_on_selected_trk
to include rackdevices. BTW I still don't understand, why we need to keep track ourselves and not just use the same self.song().view.selected_track.devices
to have realtime access to what Live is telling us.
Live has an internal concept "hard coded" that "device banks" are size 8
I am aware, there's even logic for this in our script (EC/ `if self.__chosen_plugin.class_name in list(DEVICE_DICT.keys()): ). But I also don't think that we have any problems because of this. First of all, the parameter banks are mostly/only relevant for Live's internal devices, they actually have names. Plugins do not have that concept (interesting side note: the new CLAP plugin standard, does have a concept of parameter banks), secondly I have not seen any issues in the last year w re to parameter banks.
EC line 1039
nbr_of_full_device_pages = 1
elif nbr_of_remainder_devices > 0:
nbr_of_full_device_pages += 1
I might be wrong, but my current thinking is that the elif always increments by 1 (if more than 8 devices) and that's where the error nr2 comes from. will test this theory
Update 1: nope, that's not it.
Also just tried changing EAH to if decremented_device_count_track > SETUP_DB_DEVICE_BANK_SIZE and new_current_device_bank_offset == 1:
line 418, also doesn't fix it.
The log shows nothing but should show a decrease.
When INCREASING I see EAH/device_added_deleted_or_changed: device_was_added current track device bank updated to <1>
but don't see the same when decreasing.
Update 2: didn't see it, because there was no logging for it. Added logging and pushed commit. But still not solved. Bank index remains at 1, when it should be decreased
It's not the "number of parameters in the current device bank", it's the number of banks (devices/8), right?
I didn't realize you were focusing more on the example lists themselves self.t_d_bank_current
than on how the lists are accessed, or maybe we're just hopping from rock to rock down the stream of conversation. You did mention two methods guarded by > self.t_current
Just for pedantic clarity. It helps me to think of the lists that make up (what I've been calling) the "EAH database" as "single column database tables" instead of "plain lists". Thinking about RDBMS "databases" helps to orient my thinking about the data structures in EAH (and used in EC) "vertically" (one column, zero-to-many rows) like a table structure. I naturally tend to think of lists growing "horizontally".
In that respect, the list self.t_d_bank_current
is the "table name" of the list and the "primary key" of the table is the (index of the) "currently selected track". So the name and primary key mean the table is supposed to store the "current index of" the "currently loaded bank of parameters of the appointed device on the current track". selected_device_bank_index = self.t_d_bank_current[self.t_current]
Similarly, I think of self.t_d_bank_count
such that the name and primary key mean the table stores the "number of" banks-of-eight-parameters of the "currently loaded (appointed) device on the current track". selected_device_bank_count = self.t_d_bank_count[self.t_current]
I think we're on the same page with respect to what the various "EAH database tables" are for. What I meant earlier is I wouldn't be surprised to find code I wrote (in EC maybe?) that breaks from the "banks of 8" rule and breaks something by doing device_parameters/24
or calculating a direct index without banking (example illustration only device.parameters[25]
instead of device.parameter_banks[3][1]
)
Ha. EC Line 1039 looks like code I wrote. I don't think that elif
part is related to the problem though. To really understand that logic, you need to start at the top and work down
nbr_of_full_device_pages = int(current_nbr_of_devices_on_selected_track / SETUP_DB_DEVICE_BANK_SIZE)
nbr_of_remainder_devices = int(current_nbr_of_devices_on_selected_track % SETUP_DB_DEVICE_BANK_SIZE)
if nbr_of_full_device_pages >= SETUP_DB_MAX_DEVICE_BANKS:
nbr_of_full_device_pages = SETUP_DB_MAX_DEVICE_BANKS
elif nbr_of_full_device_pages < 0:
nbr_of_full_device_pages = 0
self.main_script().log_message("Not possible, right? and yet I am logged")
if nbr_of_full_device_pages == 0 and nbr_of_remainder_devices > 0:
nbr_of_full_device_pages = 1
elif nbr_of_remainder_devices > 0: # 0 < nbr_of_full_device_pages <= SETUP_DB_MAX_DEVICE_BANKS
nbr_of_full_device_pages += 1
so "full device pages" means all 8 slots in the given "device bank" are "populated", and "remainder devices" means less than all 8 slots are populated. The if / else if at 1037 / 1039 is supposed to guard the "zero boundary" Where
if nbr_of_full_device_pages == 0 and nbr_of_remainder_devices > 0:
ensures "at least one page" always exists. You can't evaluate the elif
without taking into account exactly what won't ever be evaluated at that location. Due to the cascade of logic (if checks) above, we should always be able to assume the comment (<=
updated here) would evaluate to True
# 0 < nbr_of_full_device_pages <= SETUP_DB_MAX_DEVICE_BANKS
In other words, at the time elif nbr_of_remainder_devices > 0
is evaluated on line 1039, nbr_of_full_device_pages
is known to contain a value that is between 1 and 16. All the "remainder" += 1
accounts for is "the last page" of devices that will contain "less than 8" devices.
With all that said, IDK - your suspicions could be right on the money. Let's walk thru some scenarios.
Ha (a second time) I need to click Comment before you update again, LOL. Checking new commit(s)...
Some of your new log messages didn't appear to be formatted correctly, pushed an update. (and running tests)
FYI: With both of us working "at the same time" doing commits and pushes today, we should really each be working (doing commits) on our own local branches and using "Pull Requests" to transfer whatever "good commits" from each of our local branches to the common "GitHub origin" branch we're actively developing.
Avoidable "Merge conflict resolution" is always worth avoiding... (But on the other hand, it can be tedious to "remember and use" all that when the true need occurs so infrequently.)
For Test Case 1: At steps 3 and 4, I'm just doing Ctrl+Z for undo and CTRL+Y for redo. And I seem to be seeing latency in the LCD screen updates. What I mean is when I "undo a device insert", the "deleted device" stays on the LCD screen for about 5 seconds before it goes away, and when I redo what I just undid, the "added device" doesn't appear on the LCD screen for about 5 seconds. I'm "always" seeing this behavior, for one or more devices in the chain. Add 1, 2, 3, 4 devices; undo (wait 5 for screen update), undo (wait 5), etc. Are you seeing that same behavior?
For Test Case 1: At steps 3 and 4, I'm just doing Ctrl+Z for undo and CTRL+Y for redo. And I seem to be seeing latency in the LCD screen updates. What I mean is when I "undo a device insert", the "deleted device" stays on the LCD screen for about 5 seconds before it goes away, and when I redo what I just undid, the "added device" doesn't appear on the LCD screen for about 5 seconds. I'm "always" seeing this behavior, for one or more devices in the chain. Add 1, 2, 3, 4 devices; undo (wait 5 for screen update), undo (wait 5), etc. Are you seeing that same behavior?
Nope, (doing also key commands). For me the device STAYS in device row (row 1) (while immediately vanishing in Row 0). Live 11.2.7
Some of your new log messages didn't appear to be formatted correctly, pushed an update. (and running tests)
You're correct, I just quickly copy pasted.
FYI: With both of us working "at the same time" doing commits and pushes today, we should really each be working (doing commits) on our own local branches and using "Pull Requests" to transfer whatever "good commits" from each of our local branches to the common "GitHub origin" branch we're actively developing.
Avoidable "Merge conflict resolution" is always worth avoiding... (But on the other hand, it can be tedious to "remember and use" all that when the true need occurs so infrequently.)
Also correct, but I need to familiarize myself with cross branch pull request first. Until now, I've only done this directly on the github page when merging dev branches to master. But I will look it up! :-)
When devices are grouped in Live, you can currently only access the macro parameters of the chain's main device, but not the parameters which belong to devices which are chained within:
ToDo:
see https://github.com/markusschloesser/MackieC4_P3/blob/92ad2c62f68ef8e76b9d4279a0643bf6f7fb04e4/z_utility/tools%20MS/code_snippets_NOT_WORKING.py#L124 for possible implementation