Fix for "parameter not set" Error Message

[Martinski4GitHub]

Fixed a bug generating a "currentDIVER_version: parameter not set" error message.

[Martinski4GitHub]

@ExtremeFiretop.

A friend of mine with an RT-AX86S router had some trouble updating to the latest F/W 3004.388.10.2 version using the MerlinAU 1.5.5 version, but he soon realized that the router login password was incorrect (he had changed it last month, and he completely forgot about it), so once he corrected the password, everything went well, and the F/W was updated successfully.

However, my friend also sent me a message so I could take a look at the error he got when the F/W update was unsuccessful due to the bad login:

So I reviewed the script and found the root cause around the code that handles stopping and restarting Diversion. This bug is now fixed with this PR.

Now, I'm not quite sure if making another production release is warranted for this bug fix since this scenario is probably rare, although it does affect restarting Diversion when the error happens.

What do you think?

I'll create the PR for merging into the master branch, just in case you want to make the next production release 1.5.6 version.

There's no rush, bud, so enjoy your vacation as much as you can. Things can wait until you return.

Take care, bud!!

[ExtremeFiretop]

So I reviewed the script and found the root cause around the code that handles stopping and restarting Diversion. This bug is now fixed with this PR.

Now, I'm not quite sure if making another production release is warranted for this bug fix since this scenario is probably rare, although it does affect restarting Diversion when the error happens.

What do you think?

I guess the question is how rare is this scenario? Can you describe it in further detail?

I'm on my phone currently so my ability to analyze this is limited but I can see the code got moved up in the script.

I also know that the last time I tested MerlinAU 1.5.5 it did infact shutdown diversion for me as expected. So I would need to better understand the scenario that causes this to assess how quickly we should pump out a release.

Off the top of my head, I think the faster we get these fixes out the better, but that's always been my opinion and I think you know that. I rather have them fixed before they are reported if we know about them.

However as I joked in the forums, don't feel like you need my approval, you have the keys to the kingdom while I'm away and I trust you and your judgement 100%

Thank you for looking into this stuff while I'm away on hotel WiFi 😭

[Martinski4GitHub]

So I reviewed the script and found the root cause around the code that handles stopping and restarting Diversion. This bug is now fixed with this PR.
Now, I'm not quite sure if making another production release is warranted for this bug fix since this scenario is probably rare, although it does affect restarting Diversion when the error happens.
What do you think?

I guess the question is how rare is this scenario? Can you describe it in further detail?

I'm on my phone currently so my ability to analyze this is limited but I can see the code got moved up in the script.

I also know that the last time I tested MerlinAU 1.5.5 it did infact shutdown diversion for me as expected. So I would need to better understand the scenario that causes this to assess how quickly we should pump out a release.

Based on my review of the code, the scenario requires Diversion to be installed and a failure to log into the router WebUI. In my friend's case, it failed because of a bad password, but I suppose it could also fail for some other reason, but it may be an unusual failure.

Off the top of my head, I think the faster we get these fixes out the better, but that's always been my opinion and I think you know that. I rather have them fixed before they are reported if we know about them.

Yeah, I understand. I tend to wait a little bit to get verification from a 3rd-party, and also based on the severity of the problem. For example, if it's a consistent crash that happens, I definitely would make a release right away.

However as I joked in the forums, don't feel like you need my approval, you have the keys to the kingdom while I'm away and I trust you and your judgement 100%

Well, I only have access to half the kingdom, LOL!!

[ExtremeFiretop]

So I reviewed the script and found the root cause around the code that handles stopping and restarting Diversion. This bug is now fixed with this PR.
Now, I'm not quite sure if making another production release is warranted for this bug fix since this scenario is probably rare, although it does affect restarting Diversion when the error happens.
What do you think?

I guess the question is how rare is this scenario? Can you describe it in further detail?
I'm on my phone currently so my ability to analyze this is limited but I can see the code got moved up in the script.
I also know that the last time I tested MerlinAU 1.5.5 it did infact shutdown diversion for me as expected. So I would need to better understand the scenario that causes this to assess how quickly we should pump out a release.

Based on my review of the code, the scenario requires Diversion to be installed and a failure to log into the router WebUI. In my friend's case, it failed because of a bad password, but I suppose it could also fail for some other reason, but it may be an unusual failure.

Okay I better understand the situation now, I would have to agree that's pretty rare, but something that deserves to be flagged and fixed. I've approved the code changes in the other PR so you can merge in as you see fit.

Yeah, I understand. I tend to wait a little bit to get verification from a 3rd-party, and also based on the severity of the problem. For example, if it's a consistent crash that happens, I definitely would make a release right away.

If you feel like waiting until I return, I should be back this coming Monday, but I've approved the changes preemptively so you have nothing stopping you now.

However as I joked in the forums, don't feel like you need my approval, you have the keys to the kingdom while I'm away and I trust you and your judgement 100%

Well, I only have access to half the kingdom, LOL!!

Yes, seems like that's something else we need to investigate, I appreciate you flagging, half a key is not going to unlock a door, if I'm giving you keys while I'm gone I need to give you the full key 😂

[Martinski4GitHub]

So I reviewed the script and found the root cause around the code that handles stopping and restarting Diversion. This bug is now fixed with this PR.
Now, I'm not quite sure if making another production release is warranted for this bug fix since this scenario is probably rare, although it does affect restarting Diversion when the error happens.
What do you think?

I guess the question is how rare is this scenario? Can you describe it in further detail?
I'm on my phone currently so my ability to analyze this is limited but I can see the code got moved up in the script.
I also know that the last time I tested MerlinAU 1.5.5 it did infact shutdown diversion for me as expected. So I would need to better understand the scenario that causes this to assess how quickly we should pump out a release.

Based on my review of the code, the scenario requires Diversion to be installed and a failure to log into the router WebUI. In my friend's case, it failed because of a bad password, but I suppose it could also fail for some other reason, but it may be an unusual failure.

Okay I better understand the situation now, I would have to agree that's pretty rare, but something that deserves to be flagged and fixed. I've approved the code changes in the other PR so you can merge in as you see fit.

OK, great, thanks. I also sent my friend a message so he can verify the fix as well, although he already updated the F/W to the latest version.

Yeah, I understand. I tend to wait a little bit to get verification from a 3rd-party, and also based on the severity of the problem. For example, if it's a consistent crash that happens, I definitely would make a release right away.

If you feel like waiting until I return, I should be back this coming Monday, but I've approved the changes preemptively so you have nothing stopping you now.

Since it's not a really urgent fix, I think I'll wait until Friday evening to see if something else comes up. Otherwise, I'll merge it in and issue the next production release.

However as I joked in the forums, don't feel like you need my approval, you have the keys to the kingdom while I'm away and I trust you and your judgement 100%

Well, I only have access to half the kingdom, LOL!!

Yes, seems like that's something else we need to investigate, I appreciate you flagging, half a key is not going to unlock a door, if I'm giving you keys while I'm gone I need to give you the full key 😂

Whatever the King of the kingdom says, it goes!!! LOL!!