volumio / volumio-plugins-sources

Volumio plugins source code for Volumio 3
47 stars 250 forks source link

SmartQueue #342

Closed chourmovs closed 3 months ago

chourmovs commented 3 months ago

Hi Initial release, please review and push it to the beta channel Thank you Vincent

balbuze commented 3 months ago

Hello, Can you please close this PR and reopen one with ONLY SmartQueue in it? TY

chourmovs commented 3 months ago

Hi balbuze, I'm going to try but I know this is not as easy as it should

balbuze commented 3 months ago

in install.sh, this should not be necessary

mkdir -p /data/configuration/user_interface/smartqueue
        cp /data/plugins/user_interface/smartqueue/config.json /data/configuration/user_interface/smartqueue/config.json
        chown volumio:volumio /data/configuration/user_interface/smartqueue/config.json
        chmod 755 /data/configuration/user_interface/smartqueue/config.json

in package.json, remove "i386" not used anymore in uninstall.sh

pip remove requests
    apt-get -f autoremove python-pip -y --purge

is problematic as other plugins can use it. So remove it. No need to test if the plugin was installed.

chourmovs commented 3 months ago

mkdir -p /data/configuration/user_interface/smartqueue cp /data/plugins/user_interface/smartqueue/config.json /data/configuration/user_interface/smartqueue/config.json chown volumio:volumio /data/configuration/user_interface/smartqueue/config.json chmod 755 /data/configuration/user_interface/smartqueue/config.json

I agree but I passed my yesterday evening fixing the fact that on install, config json was not cp in /data/configuration/user_interface/smartqueue, even after a volumio updater factory This is the only workaround I found, I ask you officially if I could keep it as is for the moment I spend a lot of time to debug and test all of this

pip remove requests apt-get -f autoremove python-pip -y --purge

OK

balbuze commented 3 months ago

ok index.js, when exec(xxxxxx....) you have to handle error

let cp6 = exec("/usr/bin/pgrep shellinabox | xargs -r /bin/kill -15");
        let cp7 = exec("/usr/bin/pgrep blissify | xargs -r /bin/kill -15");
        let cp8 = exec("/usr/bin/pgrep python | xargs -r /bin/kill -15");

something like


smartqueue.prototype.onStop = function () {
    let defer = libQ.defer();

    exec("/usr/bin/pgrep shellinabox | xargs -r /bin/kill -15", (err, stdout, stderr) => {
        if (err) {
            console.error(`Error killing shellinabox: ${err}`);
        }
        exec("/usr/bin/pgrep blissify | xargs -r /bin/kill -15", (err, stdout, stderr) => {
            if (err) {
                console.error(`Error killing blissify: ${err}`);
            }
            exec("/usr/bin/pgrep python | xargs -r /bin/kill -15", (err, stdout, stderr) => {
                if (err) {
                    console.error(`Error killing python: ${err}`);
                }
                defer.resolve();
            });
        });
    });

    return defer.promise;
};
chourmovs commented 3 months ago

Thank you for this code improvement Balbuze I know that I may already have ask you for this but, with 3 plugins in beta channel, I don't know how to PR with only latest plugin code as requested, since previous beta plugin are not in official repo, this sound unfeasible if I pass by volumio plugin submit.... Maybe I should do 1 fork by plugin ?

balbuze commented 3 months ago

Try

git add  ./smartqueuefolder
git commit -m 'my comment '
git push origin master

It has nothing to do with

 volumio plugin submit

which is mandatory. Don't forget to update version number each time you submit a new version

chourmovs commented 3 months ago

git add ./smartqueuefolder git commit -m 'my comment ' git push origin master

This doesn't work, but this is not a surprise since it's already part of volumio plugin submit process... thus said, why is it so important to have ONLY SmartQueue in it ? Previous code is already in beta channel and will be part of my PR until it pass the stable channel (if I understand well) If it's important, I believe I could remove two other plugins from my repo and then PR (then only smartqueue commit will be in it) Then I could resync my repo to recover them ?, this sound painfull but OK

What do you think ? Please give me your preferences

balbuze commented 3 months ago

You have to handle error EACH time you use exec. My code was just an example... You didn't remove i386 from package.json No need for rm -r /data/configuration/user_interface/smartqueue in uninstall.sh Please test and retest x10 before repushing something. I repeat PR and plugin submit are two different things... TY

chourmovs commented 3 months ago

thank you, I will be more attentive for my next plugin submit this evening 😁 I still don't know how to PR only a part of my repo, I'm close to ask support on the forum.

chourmovs commented 3 months ago

I've applied your correction (especially exec error handling) but it brokes the plugin engine Need to rework on it

chourmovs commented 3 months ago

Ok I did slight adjustment that do the trick I totally mess trying my attempts to rebase before PR, sorry for that

chourmovs commented 3 months ago

need to provide all blissify-rs binaries for download