search

ExtremeFiretop / MerlinAutoUpdate-Router

Merlin(A)uto(U)pdate is a Merlin router script which allows you to remotely identify a stable firmware update for an ASUS Merlin router, and automatically download and update via an unattended method directly from the router.
https://www.snbforums.com/threads/merlinau-v1-2-7-the-ultimate-firmware-auto-updater-amtm-addon.91326/
GNU General Public License v3.0
21 stars 1 forks source link

Fixes, code cleanup & improvements #209

Closed Martinski4GitHub closed 6 months ago

Martinski4GitHub commented 6 months ago 
10  3  *  5-6  Sun
10  3  *  May-Jun  Sun
10  3  *  *  5-6
10  3  *  *  Fri-Sat
Martinski4GitHub commented 6 months ago

@ExtremeFiretop,

While doing more validation after making some initial code cleanup, I found problems with endless loops outputting "bad number" errors when using cron job schedules that include a range syntax for month (e.g. "5-6" == "May-Jun") or for day of the week (e.g. "5-6" == "Fri-Sat"). These are not common but they can be valid syntax.

Please take a look at the changes in this PR & run tests whenever you get a chance.

I'll be offline for the rest of the day, but I'll be back later in the evening to run a few more scenarios.

ExtremeFiretop commented 6 months ago
  • Fixed endless loops with "bad number" errors when calculating the expected F/W update run date after setting the following types of cron job schedules:
10  3  *  5-6  Sun
10  3  *  May-Jun  Sun
10  3  *  *  5-6
10  3  *  *  Fri-Sat
  • Made various code improvements & cleanup.

Great catch! I tested specific months and days not ranges. Such as:

10  3  * May,Jul 0,1
10  3  *  5,6 Sun,Mon

I'll review and test now! :)

Martinski4GitHub commented 6 months ago
  • Made various code improvements & cleanup.

Great catch! I tested specific months and days not ranges. Such as:

10  3  * May,Jul 0,1
10  3  *  5,6 Sun,Mon

Yeah, that's what I've been doing as well during my own testing; but while doing the cleanup yesterday it occurred to me that I had not tested using ranges for hours, months & days of the week so that's when I found the endless loops with the error messages. Granted, it's rather unusual to use ranges for hours, months, or days but it's a valid syntax and some user may try that.

Anyway, I finished testing with other scenarios and so far no problems at all. I just submitted a PR for a minor code improvement - nothing major. I'm about to have dinner in a couple of minutes, but after I come back I'd like to propose some changes to the top section of the Main Menu. I'll make the code changes and send you an image so you can see what I'm proposing as an alternative and we can discuss it. It's just an idea I have and you can provide feedback.

Talk to you later.

ExtremeFiretop commented 6 months ago
  • Made various code improvements & cleanup.

Great catch! I tested specific months and days not ranges. Such as:

10  3  * May,Jul 0,1
10  3  *  5,6 Sun,Mon

Yeah, that's what I've been doing as well during my own testing; but while doing the cleanup yesterday it occurred to me that I had not tested using ranges for hours, months & days of the week so that's when I found the endless loops with the error messages. Granted, it's rather unusual to use ranges for hours, months, or days but it's a valid syntax and some user may try that.

This is why 2 brains is better than one! I totally appreciate the additional logic to handle this scenario.

Anyway, I finished testing with other scenarios and so far no problems at all. I just submitted a PR for a minor code improvement - nothing major. I'm about to have dinner in a couple of minutes, but after I come back I'd like to propose some changes to the top section of the Main Menu. I'll make the code changes and send you an image so you can see what I'm proposing as an alternative and we can discuss it. It's just an idea I have and you can provide feedback.

Perfect! Happy to see otherwise everything is working flawlessly for you. Same in my most recent tests with crontab guru open on my second browser tab lol.

nothing major. I'm about to have dinner in a couple of minutes, but after I come back I'd like to propose some changes to the top section of the Main Menu. I'll make the code changes and send you an image so you can see what I'm proposing as an alternative and we can discuss it. It's just an idea I have and you can provide feedback.

Sounds good buddy! It's a long weekend for us here so I plan to stay up later tonight anyways. Just reviewing your PR now. I'm totally up to discuss for the menu I'm all ears!

Thanks again buddy! Enjoy dinner!

Martinski4GitHub commented 6 months ago
  • Made various code improvements & cleanup.

Great catch! I tested specific months and days not ranges. Such as:

10  3  * May,Jul 0,1
10  3  *  5,6 Sun,Mon

Yeah, that's what I've been doing as well during my own testing; but while doing the cleanup yesterday it occurred to me that I had not tested using ranges for hours, months & days of the week so that's when I found the endless loops with the error messages. Granted, it's rather unusual to use ranges for hours, months, or days but it's a valid syntax and some user may try that.

This is why 2 brains is better than one! I totally appreciate the additional logic to handle this scenario.

Anyway, I finished testing with other scenarios and so far no problems at all. I just submitted a PR for a minor code improvement - nothing major. I'm about to have dinner in a couple of minutes, but after I come back I'd like to propose some changes to the top section of the Main Menu. I'll make the code changes and send you an image so you can see what I'm proposing as an alternative and we can discuss it. It's just an idea I have and you can provide feedback.

Perfect! Happy to see otherwise everything is working flawlessly for you. Same in my most recent tests with crontab guru open on my second browser tab lol.

nothing major. I'm about to have dinner in a couple of minutes, but after I come back I'd like to propose some changes to the top section of the Main Menu. I'll make the code changes and send you an image so you can see what I'm proposing as an alternative and we can discuss it. It's just an idea I have and you can provide feedback.

Sounds good buddy! It's a long weekend for us here so I plan to stay up later tonight anyways. Just reviewing your PR now. I'm totally up to discuss for the menu I'm all ears!

Thanks again buddy! Enjoy dinner!

Sorry bud. During dinner, we decided to watch a movie ("Don't Worry Darling") which got interesting very early on, and it turned out to be pretty good. We're on a bathroom break right now, and we're going to watch "Midsommar" next.

Anyway, I'll modify the code & send you the image of my proposed changes by tomorrow evening. Have a good night bud.