Update MerlinAU.sh Cron Handling and Min Warning Message

[ExtremeFiretop]

Hey @Martinski4GitHub

Unfortunately the changes done in PR: #60 broke something.
I did the changes for the warning to display the min version here: 6c5045b
(It was a small change so I pushed it straight to dev).

And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Logs here for reference:
GT-AXE11000_FW_Update_2024-01-06_09_45_00.log

I rolled back the changes done in PR: #60

And everything works as a background cron job again.

[ExtremeFiretop]

I'm going to push this through now because I know it works and this is kinda important it works, but we can continue to troubleshoot if you want to identify why the $isInteractive doesn't work and the $inMenuMode does

We will need to really retest this from top to bottom if we decide to change this again.

[ExtremeFiretop]

Pushed through from Dev to Production here: #67

[ExtremeFiretop]

Now that I had a second to think since it works. I reviewed some of the code and I think the problem is that in this case, the if statement is attempting to execute the command stored in the variable $isInteractive.

    if "$isInteractive"
    then

This is not a comparison operation but an attempt to execute whatever $isInteractive contains as a command.

To fix it, we would of needed to do:

if [ "$isInteractive" = true ]; then
    # code
fi

But is there a reason to use if [ "$isInteractive" = true ]; instead of if [ "$inMenuMode" = true ];

If there is I'm all open to hearing the arguments for it!

[ExtremeFiretop]

To what I can tell, $isInteractive is set directly at the start of the script, its set to false by default, and if it detects an interactive window, it's set to true.

However $inMenuMode is set to true by default, and is set to false as soon as the arguments passed is greater than 0.
Which would be anytime we run it as a cron, which explains why it works for me when running the cron job, and works normally when running the script interactively (with no arguments passed).

So both seem they would work, one is more direct than the other, I am not sure if theres any repercussions for the way I'm doing it, I just know it works which is why I reverted as soon as I noticed the problem. However if we can get a more direct method working (i.e)

if [ "$isInteractive" = true ]; then
    # code
fi

Then I'm not against it, it just needs to be retested/validated.

[Martinski4GitHub]

Hey @Martinski4GitHub

Unfortunately the changes done in PR: #60 broke something. I did the changes for the warning to display the min version here: 6c5045b (It was a small change so I pushed it straight to dev).

And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Currently working on a fix based on a hunch... I'll explain details once I have validated & confirmed my hypothesis.

[Martinski4GitHub]

Now that I had a second to think since it works. I reviewed some of the code and I think the problem is that in this case, the if statement is attempting to execute the command stored in the variable $isInteractive.

    if "$isInteractive"
    then

This is not a comparison operation but an attempt to execute whatever $isInteractive contains as a command.

To fix it, we would of needed to do:

if [ "$isInteractive" = true ]; then
    # code
fi

But is there a reason to use if [ "$isInteractive" = true ]; instead of if [ "$inMenuMode" = true ];

If there is I'm all open to hearing the arguments for it!

No, that's not the poblem. The variable is either "true" or "false" and those are special keywords to the shell interpreter to treat it as a logical boolean. The problem lies elsewhere.

[ExtremeFiretop]

No, that's not the poblem. The variable is either "true" or "false" and those are special keywords to the shell interpreter to treat it as a logical boolean. The problem lies elsewhere.

Hey @Martinski4GitHub
Unfortunately the changes done in PR: #60 broke something. I did the changes for the warning to display the min version here: 6c5045b (It was a small change so I pushed it straight to dev).
And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Currently working on a fix based on a hunch... I'll explain details once I have validated & confirmed my hypothesis.

Take your time, and I'll gladly review and retest as soon as we push something to dev! :)

[Martinski4GitHub]

No, that's not the poblem. The variable is either "true" or "false" and those are special keywords to the shell interpreter to treat it as a logical boolean. The problem lies elsewhere.

Hey @Martinski4GitHub
Unfortunately the changes done in PR: #60 broke something. I did the changes for the warning to display the min version here: 6c5045b (It was a small change so I pushed it straight to dev).
And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Currently working on a fix based on a hunch... I'll explain details once I have validated & confirmed my hypothesis.

Take your time, and I'll gladly review and retest as soon as we push something to dev! :)

It might be some time before I can put my full attention to it. I have a plumber here this evening working on the bathtub where water has not drained since this morning so something in the sewer line is stuck, so I got to stay on top of this first.

[ExtremeFiretop]

No, that's not the poblem. The variable is either "true" or "false" and those are special keywords to the shell interpreter to treat it as a logical boolean. The problem lies elsewhere.

Hey @Martinski4GitHub
Unfortunately the changes done in PR: #60 broke something. I did the changes for the warning to display the min version here: 6c5045b (It was a small change so I pushed it straight to dev).
And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Currently working on a fix based on a hunch... I'll explain details once I have validated & confirmed my hypothesis.

Take your time, and I'll gladly review and retest as soon as we push something to dev! :)

It might be some time before I can put my full attention to it. I have a plumber here this evening working on the bathtub where water has not drained since this morning so something in the sewer line is stuck, so I got to stay on top of this first.

No worries!

Sorry to hear about the troubles with the tub, hopefully the plumber keeps his jeans up high we don't need to add the moon to our list of problems 😭 Hahaha!

I'm going to poke at it a bit more later to see if I can get a better understanding myself. But I'll wait for your word before attempting anything else, the last change was literally a copy and paste rollback just to make sure the crons continued to work.

[Martinski4GitHub]

No, that's not the poblem. The variable is either "true" or "false" and those are special keywords to the shell interpreter to treat it as a logical boolean. The problem lies elsewhere.

Hey @Martinski4GitHub
Unfortunately the changes done in PR: #60 broke something. I did the changes for the warning to display the min version here: 6c5045b (It was a small change so I pushed it straight to dev).
And then retested everything as a background cron job by downgrading to 388.4 and then setting up on a 5 minute cron cycle, it never actually updated again getting stuck at the same place as last time.

Currently working on a fix based on a hunch... I'll explain details once I have validated & confirmed my hypothesis.

Take your time, and I'll gladly review and retest as soon as we push something to dev! :)

It might be some time before I can put my full attention to it. I have a plumber here this evening working on the bathtub where water has not drained since this morning so something in the sewer line is stuck, so I got to stay on top of this first.

No worries!

Sorry to hear about the troubles with the tub, hopefully the plumber keeps his jeans up high we don't need to add the moon to our list of problems 😭 Hahaha!

I'm going to poke at it a bit more later to see if I can get a better understanding myself. But I'll wait for your word before attempting anything else, the last change was literally a copy and paste rollback just to make sure the crons continued to work.

The plumber says he's almost done. He found the problem (an old sewer pipe that got a lot of "gunk" near a joint), and he's cleaning it out with a plumbing snake that's got a camera attached to it (he even showed it to me - nice tool to work with). So another 30 minutes tops before he's completely done and cleans everything up. He's cleaning the sewer pipe all the way to the street where it joins the city's public sewer line.

[ExtremeFiretop]

The plumber says he's almost done. He found the problem (an old sewer pipe that got a lot of "gunk" near a joint), and he's cleaning it out with a plumbing snake that's got a camera attached to it (he even showed it to me - nice tool to work with). So another 30 minutes tops before he's completely done and cleans everything up. He's cleaning the sewer pipe all the way to the street where it joins the city's public sewer line.

Whoot whoot!

Well good news is once he's done and your pipes are clean you can rest easy. I've always heard clean pipes is something to be proud of and good for the health 😜

Maybe those are the wrong pipes. Either way I'm about to head out for a bit, once I'm back I'll probably have an hour or two before bed to check this out.