More Code Improvements#392
Conversation
- Added/modified the shell script to have the option to save/keep the current configuration file when uninstalling the add-on. This can be useful when a user needs to uninstalls all 3rd-party add-ons to do a full factory defaults reset, or simply wants to save the config file for re-install on a another (new?) router. - Added/modified the ASP file so the WebGUI page extracts the current version string from the shared custom settings instead of having a hard-coded string. - Some coding improvements.
Minor formatting changes.
Sorry for the delay, I had a busy weekend working for my side gig and also updating some 7 year old code for a Github project I still rely on lol. Will start to review now! |
Love this idea! Thank you!
Very cool! Will be testing this |
|
Also; what is remaining on your to-do list before we start considering wrapping it up for public testing? I would like to look into the update process from 1.3.9 and test it somehow fully. Secondly, I want to find a way to allow the user to start the update right away without needing to change the postpone period (similar to the shell script). That will take some thinking. We already have a pop-up prompt to confirm to check, so a second one to confirm to start the update anyways after the 60 second check seems a little janky to me which is the initial thought. Also want to test and confirm the Changelog approval functionality. And the ROG and TUF build features (hidden or disabled by default) |
Ah, no worries; we all have our own personal stuff to take care of first so I never expect a response right away. If I can wait 6 months for JackYaz to respond to my messages, I can wait for you a couple of weeks :>) LOL!!! |
This is what's left on my to-do list for 1.4.0 version:
I cannot really help with AiMesh Nodes or Gnuton testing but hopefully, you'll have time to do that. If I have time, I may do the first 2 items on the list. However, my workload will start ramping up very soon. The first 2 weeks of January are usually slow at work so tried to do as much as I could during that time.
Actually, the update process must be done from the 1.3.10 version because this is the one that has the latest updated code to install version 1.4.0 WebGUI as well. So, in fact, the 1.3.10 production release must be issued first (after testing & validating the upgrade process works fine) so users will be ready for the future 1.4.0 release.
What about adding a checkmark next to or below the "F/W Update Check" button to indicate that you want to ignore or bypass the postponement period and do the update right away if available?
Good points. |
All this has been noted and I agree.
I actually have been actively testing with my node and Gnuton router, so far everything with Gnuton is good, and I only noticed that the node creates the symbolic links for the WebUi even though it doesn't mount it. I am not sure if we want to be doing this on the node since the WebUi isn't being mounted anyways.
Yes fair point, the testing needs to be done against the latest dev, not production! However we would need 1.3.10 to be pushed to production ahead of time.
It's not a bad idea and was a suggestion I considered actually so probably the best course of action if we can find a way to fit it in and make it look nice.
|
OK, it's very good that you've been actively testing all this time so we know if there are any issues as soon as we can. Good job, bud!!!
Good catch!! I'll submit a PR in a couple of minutes to fix this. |
OK, I just submitted a PR that addresses the above issue WRT AiMesh nodes. |
Exactly, any test we run I run across all three environments always. It's why it takes me a bit of time to get back results sometimes. It can be some work to copy the files around on all 3 devices and mount the webUIs (or try) and collect data for the newest result. However, it's easiest to do it all together than to go back after the fact for the Gnuton and node and try to remember all the tests we did.
Merci! It was a small thing, but something I noted to modify in my next commit if it wasn't fixed already haha |
|
I have been regression testing with the latest changes, and I think that I have found a marginal timing issue with respect to validation of the Set Directory for F/W Updates field. If I enter an invalid USB directory and hit enter (or click Save), I sometimes do not see the error dialog — the invalid value is reverted to the original as it should. I cannot come to any conclusion as to when it will or will not present the error dialog. As a test for frequency, I just edited the USB location ten (10) times with invalid data, and three of the ten (3/10) times failed to display the error dialog. Each time that the error dialog was missing, the web page did not scroll down to show the Advanced Options with the Set Directory for F/W Updates field in view, but rather stayed at the top of the page. Based upon how inconsistent this is, it most likely was an existing router-specific (GT-BE98 Pro) bug that was not introduced by the recent changes. |
Excellent testing & validation strategy!! You were a couple of steps ahead all this time 👍 :>) |
Very good testing and a very good catch!! |
OK, I just committed a tentative fix for the timing issue. Whenever you get a chance, please reload/reinstall both the shell script and the ASP file before retesting. |
Installed latest fixes. Retested — ten (10) invalid USB directories resulted in ten (10) error dialogs. Good to go! |
Great news!! Just as I was about to go offline - dinner time!! |
I just attempted to recreate this specific issue with the existing version (before martinskis fix) and was unable too. Happy to hear that Martinskis fix resolved it for you though! I'll merge it in :) |
Option C: Similar to Option A, but move the label/checkbox to the right of the F/W Update Check button. Feels more intuitive to me... |
We can do that but it requires making more space between the buttons. The reason I picked the left side is because it had more space. If I space out all the buttons though I including Approve Changelog and Uninstall, we could have the space without putting things out of proportion |
How about this? It looks clean & intuitive to me as each checkmark is directly associated with the button right above so the user can enable/disable it (as needed) and then just click on the button. I'm currently coding the rest of the implementation to perform the actions required. |
I didn't try below the button! That looks good to me!! |
OK, I submitted a new PR #395 implementing the checkmarks functionality. |






Added/modified the shell script to have the option to keep/save the current configuration file when uninstalling the add-on. This can be useful when a user needs to uninstall all 3rd-party add-ons to do a full factory defaults reset and then do a reinstallation, or simply wants to save/keep the config file for re-installation on another (new?) router.
Added/modified the ASP file so the WebGUI page extracts the current version string from the shared custom settings instead of having a hard-coded value.