More script-dev questions. How to log errors in external scripts?
-
@mitu said in More script-dev questions. How to log errors in external scripts?:
OK, so if you're not storing the status in the config, then the external checks are needed. Wouldn't it be easier to read it from the config though ? - the enable/disable operations will go through your menu/gui, so you can easily control this.
I could write "enabled" to the default config and then check that to determine whether or not to enable at time of configure. For the gui though, wouldn't it still make sense to read the actual status from autostart and not just whatever is recorded in the user config? So then we are tracking every change of the enabled status what, just to check it in configure once in a while. But that's not all...
...does the enabled status even matter for configure?
If it's brand new:
- it is not enabled,
- It should be enabled though,
- So we need to enable it.
If it's old, and
...the user has not disabled it:- it should be enabled,
- It already is enabled,
- ...no action is necessary.
If it's old, and
...the user has disabled it:- it should not be enabled,
- It already is not enabled,
- No action is necessary.
It looks to me like the only time action is needed is to enable it once in the very first install and for that, we don't actually need to read any particular setting from the user config; as long as we check for it before we place it there ourselves, it should be enough just to know if the file exists at all (no action) or not (enable first time).
I was thinking that for normal operation loading the RetroPie
packages
functions wouldn't be needed, this involves additional function executions (i.e. evaluate the system platform, read packages, find thegui_
function, etc.) that shouldn't be needed just for running Next/Prev/etc.But the es_systems command for all of retropiemenu is just
retropie_packages.sh launch %ROM%
. Even with an .sh file I think I can't not use it, unless like I put the .sh script in ports instead (where the command is simplybash %ROM%
).*Just have in mind that on other systems (rpi0/rpi3, slower sdcard) this time may increase.
Well, that is something to consider I suppose, but also it's not meant to be a full-fledged media player (just install Kodi or something). The core function is meant to be the automatic playing, pausing, resuming, and killing of the music player in the background. The manual pause and skip track options are just "bonus content," if you will.
EDIT: *Oh...! Maybe it could be it's own whole system. Have individual .sh "roms" for every play function, that launch with bash, then just have one that calls
retropie_packages.sh gui
for the "edit configuration" item....but that's a project for a future day, I think.
-
...
It looks to me like the only time action is needed is to enable it once in the very first install and for that, we don't actually need to read any particular setting from the user config; as long as we check for it before we place it there ourselves, it should be enough just to know if the file exists at all (no action) or not (enable first time).Yes, that's correct. On upgrades though, why do you need to re-read the configuration and invoke enable/disable ?
But the es_systems command for all of retropiemenu is just retropie_packages.sh launch %ROM%. Even with an .sh file I think I can't not use it, unless like I put the .sh script in ports instead (where the command is simply bash %ROM%).
Yes, you're right - I forgot that only the ports run
bash
only. Maybe something that could be improved here. -
@mitu said in More script-dev questions. How to log errors in external scripts?:
On upgrades though, why do you need to re-read the configuration and invoke enable/disable ?
On upgrades, we don't need to enable or disable, just leave it alone. That's why we need to check (one way or another): 'is this an "upgrade" configure, or an "install" configure.'
...is there an
[[ "$md_mode" == "upgrade" ]]
?Yes, you're right - I forgot that only the ports run
bash
only. Maybe something that could be improved here.See my edit.
-
@sleve_mcdichael said in More script-dev questions. How to log errors in external scripts?:
On upgrades, we don't need to enable. That's why we need to check: is this an "upgrade" configure, or an "install" configure.
...is there an [[ "$md_mode" == "upgrade" ]]?No, but you can always check for the config file existence to determine if it's a new install and use a
toggle_bgm "on"
beforecopyDefaultConfig
(toggle_bgm
would be the function that enables/disables the module, with the on/off param). -
@mitu said in More script-dev questions. How to log errors in external scripts?:
you can always check for the config file existence to determine if it's a new install and use a
toggle_bgm "on"
beforecopyDefaultConfig
Yeah that's how I've done it. I guess I hadn't published this version yet:
Do you think the
sed
and/ortouch
should get moved from install to configure? -
@sleve_mcdichael said in More script-dev questions. How to log errors in external scripts?:
Do you think the sed and/or touch should get moved from install to configure?
No, they can stay there.
-
@mitu here is where I am at after last night:
I made the assignment of the "global-ish" vars look "prettier." I put them all in one function, with the
$1
param dictating what should be returned. Then I put declaration and assignment on one line (still as two statements, with a&&
in between. Makes them wider and less tall, less "towering.") This seems to be working, does it break any of the rules? Definition and asignment...
Re: onstart/onend. In case the user already has other actions here, do you think the fade-out/in should be the first thing that happens on game start and the last thing that happens on return, or some other combination?
...
I said:
So then we are tracking every change of the enabled status what, just to check it in configure once in a while. (...) does the enabled status even matter for configure?
Turns out, maybe. remove calls disable first, for obvious reasons. So, if you remove then reinstall, it currently doesn't get re-enabled, because it 1) has been installed before, and 2) has subsequently been disabled. So maybe recording this status in user config and using that to determine what to do at configure would be better, after all. I'll write the status change in the gui action so it's not recorded when uninstall calls the disable function directly. Then reinstall will still see the "enabled" config and know to take the necessary action.
...
I said:
And sometimes the track doesn't start up the first time you hit "play" after re-enabling it.
I discovered this in the stand-alone menu script but it appears is present also in the current GUI-only iteration. I've at least identified how and when this happens, if not why. If I PAUSE the music, then DISABLE the music, then ENABLE the music, then PLAY the music, it doesn't start, and I have to hit PLAY/PAUSE a second time.
Experimenting, I found that if I DISABLE while the music is playing, then SSH in and
pgrep mpg123
I get nothing. But if I PAUSE it first and then DISABLE, I still get a PID.If I
ps -ostate= -C mpg123
I can see that it is:S (interruptible sleep) while playing, and
T (stopped) while paused.If I pkill while it's "sleeping" (playing) it works fine, but if I pkill while it's "stopped," nothing. No error message, it's just still there after.
sudo pkill
didn't work either but using SIGKILL (pkill -kill mpg123
), did.In the stop script I am using
pkill
(SIGTERM). I have read that SIGKILL should only be used as a last resort when nothing else works, so I am seeking alternate solutions.Also, I'm still not sure why this causes it to do nothing when I hit "play" the first time. Since the process is still there and stopped, shouldn't the fade script still just restart it with
-CONT
, just like after it's paused? What does the (failed) attempt at killing the process do, that prevents it from restarting the first time but works fine the next time? -
@sleve_mcdichael said in More script-dev questions. How to log errors in external scripts?:
I made the assignment of the "global-ish" vars look "prettier." I put them all in one function, with the $1 param dictating what should be returned. Then I put declaration and assignment on one line (still as two statements, with a && in between. Makes them wider and less tall, less "towering.") This seems to be working, does it break any of the rules? Definition and asignment
You can use direct assignment as-well (
local var=<value>
) and it's even shorter. Doesn't break any rules AFAIK.If I pkill while it's "sleeping" (playing) it works fine, but if I pkill while it's "stopped," nothing. No error message, it's just still there after. sudo pkill didn't work either but using SIGKILL (pkill -kill mpg123), did.
For
mpg123
related stuff, I don't have any experience with it. Maybe using a FIFO named pipe for commands (similar to how RetroArch accepts commands via the network interface) is more robust, since you're not relying on OS signals to control the instance, but send full commands instead. Look up the--fifo
parameter and see if it works better. -
@mitu said in More script-dev questions. How to log errors in external scripts?:
You can use direct assignment as-well (
local var=<value>
) and it's even shorter. Doesn't break any rules AFAIK.local autostart="$(_get_vars_bgm123 autostart)"
...won't get me in trouble with the Style Police because it's a command substitution?
Declaration and assignment must be separate statements when the assignment value is provided by a command substitution, as the local builtin does not propagate the exit code from the command substitution.
# DO NOT do this: $? contains the exit code of 'local', not my_func local my_var="$(my_func)" [[ $? -eq 0 ]] || return
Of course I'm not making any use of the command's exit codes so if that's the only reason to avoid this, it may not apply here. But that's the only reason I didn't do this in the first place; if people who know more than me think this is safe I would go ahead and use it.
Edit: I see this is actually used repeatedly in
local choice=$("${cmd[@]}" "${options[@]}" 2>&1 >/dev/tty)
in gui functions (including the one I copy/pasted from another module to build this one off of) and a handful of other uses including some that look an awful lot like what I'm doing so I'ma go ahead and go with it. -
@sleve_mcdichael said in More script-dev questions. How to log errors in external scripts?:
...won't get me in trouble with the Style Police because it's a command substitution?
Arguably, since you're using
"$(my_func)"
instead of a hashmap (hint!). In any case, if you'd like to separate declaration from assignment, then the&&
doesn't make much sense, sinelocal
is not a command that may faillocal var; var = "$(my_func)"
-
@mitu said in More script-dev questions. How to log errors in external scripts?:
Arguably, since you're using
"$(my_func)"
instead of a hashmap (hint!).Is that the same as an "associative array"?
function _get_vars_bgm123() { declare -A gvar=( [autostart]="$configdir/all/autostart.sh" [bashrc]="$home/.bashrc" [onstart]="$configdir/all/runcommand-onstart.sh" [onend]="$configdir/all/runcommand-onend.sh" [autoconf]="$configdir/all/$md_id.cfg" [menudir]="$datadir/retropiemenu" [init]="$md_inst/bgm_start.sh" [killscript]="$md_inst/bgm_stop.sh" [fadescript]="$md_inst/bgm_fade.sh" ) echo "${gvar[$1]}" }
I've just done this with no other changes and it didn't break anything. (Well it broke a lot, because I made some typos, but after I cleaned up that mess and fixed the code, it all works fine again.)
I'm still just
echo
-ing the value, and assigning and using them in functions with"$(my_func)"
exactly as before. I experimented with just calling the "get_vars" function once and then using${gvar[foo]}
directly but this seemed to work much likelocal
vars, in that the array doesn't seem to exist outside the function:pi@retropie:~/temp/foo $ cat test.sh #!/bin/bash function get_vars() { declare -A global_var=( [key1]="value1" [key2]="value2" ) echo "${global_var[key1]}" } get_vars echo "${global_var[key2]}" pi@retropie:~/temp/foo $ bash test.sh value1 pi@retropie:~/temp/foo $
EDIT: This looks promising:
pi@retropie:~/temp/foo $ cat test.sh #!/bin/bash function get_vars() { declare -A global_var=( [key1]="value1" [key2]="value2" ) for f in ${!global_var[@]}; do echo "local $f=${global_var[$f]}" done } function test() { $(get_vars) echo "$key1" echo "$key2" } test pi@retropie:~/temp/foo $ bash test.sh value1 value2 pi@retropie:~/temp/foo $
-
@sleve_mcdichael Yes, this looks ok.
You may want to remove the file backup stanza inconfigure
, since you're already doing it inside thetoggle_
function when you call it later. -
More lines again but I feel like it's cleaner, code-wise:
@mitu said in More script-dev questions. How to log errors in external scripts?:
@sleve_mcdichael Yes, this looks ok.
You may want to remove the file backup stanza inconfigure
, since you're already doing it inside thetoggle_
function when you call it later.The idea here was to have a permanent "archival" backup of the original files, and also an immediate backup of the previous state from before it was toggled.
Mostly I just wanted to make sure I had a way out if I goofed something up. Since the toggle function does seem to reliably be adding and removing only the selected lines, though, maybe that backup isn't really necessary and I should only keep the first one in configure. That way, whatever happens to the files, one can always go back to how they were before it was ever installed. And the "toggle" backup doesn't really help much, it's exactly what you'll get if you just toggle it again anyway.
Contributions to the project are always appreciated, so if you would like to support us with a donation you can do so here.
Hosting provided by Mythic-Beasts. See the Hosting Information page for more information.