Skip to content

Dev 1.0.3 Target as Next Stable Release#117

Merged
ExtremeFiretop merged 13 commits intomainfrom
dev
Feb 10, 2024
Merged

Dev 1.0.3 Target as Next Stable Release#117
ExtremeFiretop merged 13 commits intomainfrom
dev

Conversation

@ExtremeFiretop
Copy link
Owner

@Martinski4GitHub

Dev 1.0.3 Target as Next Stable Release

  1. Moved a function for cleanliness.
  2. Added a new email notification for failed checksum
  3. Decided to stop toggling LEDs while a Backupmon update is in progress

@ExtremeFiretop ExtremeFiretop added the enhancement New feature or request label Feb 9, 2024
@ExtremeFiretop ExtremeFiretop self-assigned this Feb 9, 2024
Copy link
Collaborator

@Martinski4GitHub Martinski4GitHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to go.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Feb 10, 2024

@Martinski4GitHub

Okay I fixed/addressed the issue brought up here: https://www.snbforums.com/threads/introducing-merlinau-the-ultimate-firmware-auto-updater-addon.88577/post-891658

In Commit: 4ecfef0

and

7d74c89

I am going to wait for your review because I did have a drink or two tonight and you may have a better way to match the files. (Thats why it's 2 commits after all) But I tested this method though and it works! :)

Also addressed BACKUPMON nvram differently, I moved BACKUPMON before the blink function starts, so it doesn't pause midway through the blink cycle. Tested and it works as well!

Have a goodnight buddy. Much love!

- Fixed trap statement.
- Additional check in "email notification" setup.
- Fixed the "find" statement to get the Changelog file of the firmware to be flashed.
- Double-quoting variables & assignment statements per coding standard.
MerlinAU.sh Outdated
# Define the path to the log file
changelog_file="${FW_BIN_DIR}/Changelog-NG.txt"
#Files matching the patterns 'Changelog-NG.txt' or 'Changelog-386.txt'
changelog_file=$(find ${FW_BIN_DIR} -type f \( -name "Changelog-386.txt" -o -name "Changelog-NG.txt" \) -print | head -n 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "find" cmd statement uses the "-type" argument which is not valid with the built-in "find" command. It's valid with the Entware "find" utility which not all users may have installed. Also, I prefer to search for the pattern "Changelog-*.txt" which will find possible future changes in the name such as "Changelog-388.txt" in addition to current file names.

@Martinski4GitHub
Copy link
Collaborator

Martinski4GitHub commented Feb 10, 2024

@Martinski4GitHub

Okay I fixed/addressed the issue brought up here: https://www.snbforums.com/threads/introducing-merlinau-the-ultimate-firmware-auto-updater-addon.88577/post-891658

I fixed the "find" cmd statement in my PR because it was using an argument that is not valid with the built-in "find" command. See my comment for more details.

Also addressed BACKUPMON nvram differently, I moved BACKUPMON before the blink function starts, so it doesn't pause midway through the blink cycle. Tested and it works as well!

Sounds good.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Feb 10, 2024

@Martinski4GitHub
Okay I fixed/addressed the issue brought up here: https://www.snbforums.com/threads/introducing-merlinau-the-ultimate-firmware-auto-updater-addon.88577/post-891658

I fixed the "find" cmd statement in my PR because it was using an argument that is not valid with the built-in "find" command. See my comment for more details.

Also addressed BACKUPMON nvram differently, I moved BACKUPMON before the blink function starts, so it doesn't pause midway through the blink cycle. Tested and it works as well!

Sounds good.

Oh buddy this is why I wait for you!

Let me test this and review, on the surface not only does it look better but it just makes more freaking sense your way! 🤣

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Feb 10, 2024

@Martinski4GitHub

That was super slick, because your PR was merged before mine into dev , my PR just picked up yours and updates/syncs the changes.

I was thinking about conflicts but I'm syncing into main instead of dev so, zero conflicts!
Your changes just squeezed right in! I tested everything through and it work A+, thank you for addressing the changelog and for fixing the trap and all the little cleanups and checks along the way!

@ExtremeFiretop
Copy link
Owner Author

Yep, tested fully now, your changes make me much more comfortable merging this in.

@Martinski4GitHub moving forwards it's back to dev to dev changes first, this one was just a bit of a last moment squeeze in!

@ExtremeFiretop ExtremeFiretop merged commit 4888511 into main Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants