fix nxdnparrot not starting with just dmr2nxdn enabled

[narspt]

For this commit I did just copy exactly what you have on nxdngateway.service ... but tbh I really don't like it :) Why don't you do something more simple and clear like:

if [ `sed -n '/\[NXDN Network\]/{n;p;}' /etc/mmdvmhost | cut -c 8` == 0 ]; then
    if [ -f '/etc/ysf2nxdn' ] && [ `sed -n '/\[Enabled\]/{n;p;}' /etc/ysf2nxdn | cut -c 9` != 1 ]; then
        if [ -f '/etc/dmr2nxdn' ] && [ `sed -n '/\[Enabled\]/{n;p;}' /etc/dmr2nxdn | cut -c 9` != 1 ]; then
            exit 1;
        fi
    fi
fi

I don't see the need for the test=pass and redundant if's that make it more complex... and when you add a 3rd converter app there it will get much worst... :)

[AndyTaylorTweet]

It is getting complex and ugly.

So in your solution if NXDN is off in /etc/mmdvmhost and the file /etc/ysf2nxdn doesn't exist for some reason (like the bins are updated but the Upgrade has not run yet) the test is skipped and the service starts.

If you can make up some alternative rules to make sure that the service behaves as it should, and ONLY starts when it should - thats fine.

[AndyTaylorTweet]

my "test=pass" is just a thing to do, so that the if chain continues, bash doesn't like to have nothing to do there.

[narspt]

hmm, yes I see it now, sorry, but it's really ugly indeed :)

[AndyTaylorTweet]

It is - and I know its offensive to those of us with CDO (its like OCD, but I put the letters in the correct order);

If you come up with a more pretty method - I would of course be interested :)

[narspt]

Ok, fixed version of my suggestion:

if [ `sed -n '/\[NXDN Network\]/{n;p;}' /etc/mmdvmhost | cut -c 8` == 0 ]; then
    if [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/ysf2nxdn 2>/dev/null | cut -c 9`" != 1 ]; then
        if [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/dmr2nxdn 2>/dev/null | cut -c 9`" != 1 ]; then
            exit 1;
        fi
    fi
fi

There would be surely a lot of other ways to do it... but anyway note that this is just that - a suggestion - as I think your way may get really complex and bug prone mainly if/when there are more converters in the equation... you see what I mean :)

[AndyTaylorTweet]

I've been trying to find a problem with this.... but I can't, I think that would work, and it is MUCH cleaner than mine.

[narspt]

Another way (keeping your if lines intact) would be:

if [ `sed -n '/\[NXDN Network\]/{n;p;}' /etc/mmdvmhost | cut -c 8` == 0 ]; then
    for _ in once; do
        if [ -f '/etc/ysf2nxdn' ] && [ `sed -n '/\[Enabled\]/{n;p;}' /etc/ysf2nxdn | cut -c 9` == 1 ]; then
            break;
        fi
        if [ -f '/etc/dmr2nxdn' ] && [ `sed -n '/\[Enabled\]/{n;p;}' /etc/dmr2nxdn | cut -c 9` == 1 ]; then
            break;
        fi
        exit 1;
    done
fi

longer... but IMO cleaner as well... :)

[AndyTaylorTweet]

I should always match on "1" only too, because there can be some old legacy cases where the config files / lines dont exist.

so either ==1 or !==1, although the exact match to "0" will only match of the file / lines exist, since if the file is not present the output is null, either way I need to make sure I dont match on !==0

[narspt]

You will never get a null type here, but just an empty string. For the case where file / lines doesn't exist the quotes like "`...`" I added on my shorter sample are important due to that, else it fails as there will be nothing before the comparation operator. And surely quotes can also be added on the longer sample one to also handle files existing but without matching lines (that one already handles non existing files with -f check).

[narspt]

And I didn't touch your mmdvmhost check... but also improving that:

if [ "`sed -n '/\[NXDN Network\]/{n;p;}' /etc/mmdvmhost 2>/dev/null | cut -c 8`" != 1 ]; then
    if [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/ysf2nxdn 2>/dev/null | cut -c 9`" != 1 ]; then
        if [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/dmr2nxdn 2>/dev/null | cut -c 9`" != 1 ]; then
            exit 1;
        fi
    fi
fi

i.e. only continue the script (without reaching the exit) if any is explicitly set to 1 (0 or empty string due to missing file/lines is !=1 and if all like that then it should reach the exit) or...

for _ in once; do
    if [ -f '/etc/mmdvmhost' ] && [ "`sed -n '/\[NXDN Network\]/{n;p;}' /etc/mmdvmhost | cut -c 8`" == 1 ]; then
        break;
    fi
    if [ -f '/etc/ysf2nxdn' ] && [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/ysf2nxdn | cut -c 9`" == 1 ]; then
        break;
    fi
    if [ -f '/etc/dmr2nxdn' ] && [ "`sed -n '/\[Enabled\]/{n;p;}' /etc/dmr2nxdn | cut -c 9`" == 1 ]; then
        break;
    fi
    exit 1;
done

[AndyTaylorTweet]

Please submit these as pull requests, and add to the banner at the top "improved by..." etc, its important to credit your work :)

[narspt]

Well I don't know what one you prefer :) And concerning credits, it's just a small suggestion... but thanks :)

[AndyTaylorTweet]

The 2nd option is the best idea, since it will allow for further expansion later :) Small ideas can lead to big things and I appreciate your help :)

[narspt]

done :)

[AndyTaylorTweet]

Thank you!