[01:23] Eickmeyer: in your comment on the - no remove install media - bug, you did not indicate if the test install was to metal or a virtual setup. The remove media message seems to show up in the virtual case where it is not needed, but not on a hw install. so if it worked on a hw install, great fixed! otherwise... probably still broken [01:24] OvenWerks: It was a hardware install, so definitely fixed. [01:27] great! I think I saw a similar report for xubuntu as well [01:28] OvenWerks: Well, it seems that Ubuntu proper and Ubuntu MATE are affected, so there are likely to be more spins. [16:34] The use of globals in autojack makes me sad [16:34] I don't think I want to address that right now, however [16:45] wonko: the idea of giving each call the whole list has it's drawbacks too. [16:46] That's what classes are for. :) [16:46] wonko: however, it is a good idea to keep each commit to small spaces [16:46] hmm small changes maybe? [16:47] not just commit, but block of work. I'm trying to maintain the scope of just adding configparser support and nothing else (and maybe cleaning up some formatting/PEP8 nonsense as I go along) [16:47] maybe so the pep stuff first [16:48] that way the next change is just that [16:48] That is get any formatting changes over with in one commit before doing anything else [16:48] it makes the diff a lot easier to read [16:48] Yeah, that probably would have been best. PyCharm does all the heavy lifting though so maybe if I do a quick branch to update that and merge the changes into configparser we'll get a more accurate showing of changes [16:49] heheh fun fact about Pycharm [16:49] you can make many commits :P [16:49] if it has git integration ;) [16:49] so when you're done iwth say formatting, you just commit *that* [16:49] then you make more changes, then commit *those* [16:49] *uses PyCharm religiously for major Python projects* [16:50] ... you can do all this manually too but :P [16:50] anyways I digress :) *lurks* [16:51] teward001: It's not just commits though (and yes, I hit the commit checkmark button *constantly* in PyCharm) [16:52] It's trying to keep the scope of the change in control [16:52] this branch is for adding configparser support, so it should really only have commits related to that [16:52] ah. well i'd still fix formatting things *first* ;) [16:52] but i digress [16:53] right, which is what we were just talking about. I'll do a separate branch to handle that [16:53] then merge it into my configparser branch to get back on track with scope [16:53] Also I'm going to start calling you jayztwocents now Mr. I Digress. :-P [16:54] lol [16:54] be glad you caught me when i'm caffeinated [16:54] of course from my POV it is "break" formatiing first. Python formatting is :P [16:54] Erich has seen me when i have NO caffeine, and can attest to the evil there :) [16:55] OvenWerks: python formatting is worse than that. It's one of the things that kept me from Python in the first place and it's something I still hate. [16:55] ... though tsimonq2 can be equally well aware of my evil uncaffeinated state :P [17:23] OvenWerks: https://code.launchpad.net/~ubuntustudio-dev/ubuntustudio-controls/+git/ubuntustudio-controls/+merge/374098 [17:24] Nothing should have changed, but I'd like you to eyeball this real quick just to make sure it looks ok before we merge it. [17:25] erm [17:25] why is `.idea` not in your gitignore? [17:25] it is [17:25] then your git is broken [17:25] because it included .idea/codeStyles/* and such [17:25] in the diff [17:25] oh crap, I commited before adding the .gitignore. Let me clean tha tup [17:26] .idea/.gitignore (+3/-0) .idea/codeStyles/codeStyleConfig.xml (+5/-0) .idea/inspectionProfiles/profiles_settings.xml (+5/-0) .idea/misc.xml (+7/-0) .idea/modules.xml (+8/-0) .idea/ubuntustudio-controls.iml (+11/-0) .idea/vcs.xml (+6/-0) [17:26] that's the stuff that was still included :P [17:27] NACK on the diff as is, because BadFilesIncluded [17:27] yeah, PyCharm added that before I added the gitignore [17:27] wonko: it looks like you would like to do a reset? [17:27] yeah i'd start the diffset over again [17:28] Yeah, let me nuke and start over. Didn't actaully spend effort coding, so it's not hard. :) [17:28] wonko: it doesn't look like you starting at where master is now? [17:29] it was a branch of master, so it had better be [17:29] Yup [17:30] Sorry I was looking at configparser from last week [17:30] ah, right, that's why it lets me reject the proposal... I forgot taht Erick added me to the Ubuntu Studio Dev team, and my Core Dev status also gets me in the team. Was wondering why LP let me do that xD [17:31] Erich* [17:32] Because reasons. [17:32] lol [17:33] true. This said, i forgot that that gives me elevated access xD [17:33] smdh [17:33] ... well Core Dev would've gotten me that too [17:33] left over from zequence a lot of it. [17:33] ^ [17:33] I'm STILL cleaning up his mess. [17:34] OvenWerks: I hope you don't mind me stepping in and ursurping the requested review on the merge proposal because of the cruft in it xD [17:34] i usually don't like doing that but in *this case*... :) [17:34] I'm all for it [17:34] catch my nonsense, please. :) [17:35] that's what Erich told me to do when he added me to the dev team xD [17:35] not a problem... I am waiting for the dust to settle before I do anything more at all [17:36] @teward001 FYI, none of these changes are for this cycle (should be obvious) but for next cycle. [17:37] https://code.launchpad.net/~ubuntustudio-dev/ubuntustudio-controls/+git/ubuntustudio-controls/+merge/374100 [17:37] that should be better [17:37] OvenWerks: Yep! [17:37] we're waaaay past the point of getting things in :P [17:38] eww those subprocess.run calls could use some `shlex` refactoring >.< [17:38] just saying :P [17:40] wonko: as a pythoner, LGTM, but it's up to OvenWerks to ack the merge :P [17:40] i just step in to deny things when it makes sense to xD [17:41] BRB work meeting. [17:41] teward and his ban hammer [17:42] lol [17:42] you asked for the review ;) [17:42] *returns to work* [17:43] https://i.redd.it/iy4iri8m63r21.jpg <-- actual picture of teward [17:44] have no idea what he means by "subprocess.run calls could use some `shlex` [17:44] refactoring" [17:45] getting the things to work was already painful. They all used to be single string commands sent to bash or sh. [17:45] (which just worked) [17:46] OvenWerks: the way it's currently split into bits in the array can be done as a single string passed to `shlex.split(...)` but that's for later consideration :) [17:46] just things about how SP calls are in there tahat irk me ;) [17:46] but not critical :) [17:46] what is SP calls? [17:47] SP = subprocess [17:47] subprocess.run(["command", "here", "with", "args"]) == subprocess.run(shlex.split("command here with args")) [17:47] i'm just picky and annoyingly so :P [17:47] but as i said [17:47] not critical nor does it really need refactoring to be Good 2 Go [17:47] That would mean making the string first so same difference. [17:48] but the no-op PEP8 changes look good as is to me :) [17:48] but again, that one's not my call :) [17:48] So long it still runs is all I care [17:48] ... jeez my boss is late to the meeting xD [17:48] Thats what bosses are for [17:49] (making the little people wait...) [17:51] OvenWerks: I pushed those to files to /usr/bin on my machine and everything is working as expected (un-surprisingly) [17:51] s/to/two/ [17:51] So this is now before config parser? [17:51] yes [17:51] this is without config parser changes [17:52] this is just formatting changes branch [17:52] so that should be good for a merge as far as I know [17:53] teward: Also, this is why I don't like doing straight merges to master. You caught a mistake. Merge request system FTW. :) [17:53] Ya I think so [17:56] probably for mere formatting changes a changelog entry was not needed. But leave it in anyway. [17:57] Probably, but it seemed like the right thing to do [18:01] Ok so how do I merge (online)? Or do I just clone and merge offline? [18:06] I've never used launchpad before, so I don't really know, but there should be a way for you to approve the merge [18:06] and then there should be a way for you or I to apply the merge within launchpad itself. [18:07] I can click on the pencil next to Needs review and get the list of options, maybe that's where you do it? [18:07] not gonna lie, launchpad is hella janky and I don't like it. :) [18:07] I can't find it... and the command they give doesn't work either... give me a minute or two. I am used to working with the PR method... [18:08] This is a PR [18:08] just with a different name [18:08] https://imgur.com/kCPGDrk.png [18:08] There, where it says Status [18:08] I think that's where you change it? [18:08] * wonko dreams of gitlab [18:13] Actually I should be able to just switch to the remote branch [18:15] don't do it outside of the merge system, that defeats the purpose. :) [18:16] Ok, it shows as approved [18:16] but now I'm not sure what to do. [18:21] It says merged [18:22] I had to do a pull first so My system knew of the new branch, then I could do checkout branch [18:22] merge and push [18:23] hmm, ok [18:23] like I said. Hella Janky. :) [18:24] so if you look at: https://code.launchpad.net/~ubuntustudio-dev/ubuntustudio-controls/+git/ubuntustudio-controls/+ref/master [18:24] under recent commits? [18:24] it says merged from branch.... [18:24] yeah, guess it does some behind the scenes tracking [18:24] ok, at least we know how that works now [18:24] So yu can remove that branch now. [18:25] deleted [18:25] is configparser finished now or does it need redoing? [18:26] I need to merge the new master into it and finish up the work on autojack [18:26] ok [18:38] Ok, master merged into configparser branch, now, where was I? :) [19:14] OvenWerks: don't know if you know the answer to this or not, but in get_dev_info() is that usb variable supposed to be the global one? [19:15] wonko: no [19:16] in get dev info I think it is about if that device is a usb device [19:16] The use of global variable names as local variable names is really, really bad. :( [19:17] I would have to look... but not right now, the friendly mail lady just dropped off a 1394a PCIe board so I am part way through shutting down... back in a bit [19:17] no rush [19:17] just verifying what I was pretty sure I already knew [19:24] Ugh, not I'm not sure if I overwrote local variables or not. I should probably go back and start over. bleh. [19:36] Ok, I'm back to before I started today. And looking at this code again with the new vision of poor variable naming I have to say I'm surprised this shit works at all. :) [19:36] REWRITE ALL THE THINGS! :P [19:47] wonko: there is that [19:49] wonko one step at a time. [19:57] So, I don't use globals often. If you declare something global in one spot but not about what happens? The one function is using pulse_in etc without declaring it global. [19:57] Ya, There are some things like that which need fixing [20:00] python has this thing where if you read a variable that has not been set in scope it looks in higher scopes. However, if you write to it, it becomes local. [20:01] Fun, yes? [20:04] Yeah, fun. Totally the word I'd use. 🤣 [20:05] In my last pass through things, I did add a number of "global " lines, but obviously not all. You are fixing this for a large part with config which stores most of the globals anyway. [20:07] Yeah, I just need to make sure I don't use that where it shouldn't be [20:58] So I have three items... at least one of them does not work but I do not know which item it is :P [21:24] Ok, I think I've found all occurences of local variables that shadow globals and have renamed them with a l_ prefix. [21:24] which should keep me from squishing them (which, as it turns out, I totally was) [21:28] OvenWerks: I have a question about this: https://gist.github.com/bhechinger/f8b4468234411fcd6f11ca2c0412561b [21:28] I don't remember seeing that in controls. What really should I be doing here? [21:30] Yeah, checking master, in controls it was this: https://gist.github.com/bhechinger/f13b718aa2057b743b9debfa5a60b59c [21:30] And right there is the danger in having the same code copied all over. :) [21:52] that is an error which should be fixed to read dev = "0,0,0" because it should be a string [21:52] but which is right? [21:52] setting that to 0,0,0 or not? [21:52] because it goes both ways [21:52] controls doesn't change it to 0,0,0 if it's set to default and autojack does [21:53] auto jack has to because it has to be a device we can get from parsing /proc/asound [21:54] controls wants to show "default" to the user... so they know they havn't change it from whatever the value was we tried. [21:54] ah, ok, that makes sense [21:54] thanks [21:54] but in any case they are both strings [21:54] yet another anomaly to deal with. :) [21:55] yeah, that not having quotes was my next question but you already answered that. :) [21:56] Another thing I noticed. In reconfig() there is this: [21:56] if pulse == "True": [21:56] connect_pa() [21:56] but pulse won't normally get set to true [21:56] wonko it would be ok to deal with it by using one varialbe for the config value and another for the massaged value or do the change at the point it is used. [21:56] so, uhm, what? [21:57] OvenWerks: I think it makes sense to do like we're doing with zdev. It only matters in the scope of the internals of autojack, so let's do the needful there if we need to. [21:58] pulse should be a string "True" as it is read from the config. [21:58] it is not a bool [21:58] except that is legacy config that shouldn't exist anymore [21:59] that gets converted into PULSE-IN/PULSE-OUT = 0/1 [21:59] do we set it close by? [22:00] it's only set if reading in the config sees PULSE, which it turns into those other two and then doesn't write PULSE back out [22:01] that looks like an error [22:02] I assume you mean autojack line 348 [22:02] yes [22:03] That will have to be changed... or reused [22:03] I would suggest reusing it [22:04] set it to true if either PULSE-IN or PULSE-OUT aren't 0? [22:04] yes [22:04] ok, i'll tweak that then [22:05] also when we add naming it will have see that neither pulse_in or pulse_out are []. [22:05] right [22:05] which may work out to bool of sorts. [22:06] if def_config['PULSE-IN'] == "0" && def_config['PULSE-OUT'] == "0": [22:06] pulse = "False" [22:07] uh, and [22:07] not && [22:07] :-D [22:07] sure so long as it is already is True [22:07] Yes, true is the default [22:08] That way, only the one call really needs to know what pulse_in/out are. [22:11] I want to change the config call so we only do it once. Is there a way to copy/rename the current config.* to oldconfig.*? [22:15] That should be possible, yes [22:15] but let's not change stuff like that just yet [22:15] because I need to make sure I don't break things as it is. :) [22:15] which isn't easy with this code [22:15] ya :) just thinking out loud [22:16] At this point I can't promise I didn't break anything. ;) [22:17] This is a good time to break things [22:24] I normally start autojack from the commandline and add print() statements in many places to check things are doing what I think they should be when testing. [22:33] Ok, so I think I managed to do this without breaking things. Maybe. :) [22:33] Do you have a set of regression tests you use? Or is it just seat of the pants sort of stuff? [22:36] Ok, well it explodes immediately. I'll fix that. :) [22:43] Ok, explosion fixed [22:44] it doesn't create the pa bridges though [22:44] so that's the next thing to deal with, but that's for tomorrow [22:44] code is checked in if you want to poke at it yourself