Suggestion 1: Silence AP Login Attempt Failures#484
Suggestion 1: Silence AP Login Attempt Failures#484ExtremeFiretop wants to merge 3 commits intodevfrom
Conversation
For AiMesh nodes, the password is always the same as the primary with no options to change, so we don't need the passwordloginchecker helper for those. And for an AP, they can be set to any password that doesn't match the primary, so displaying invalid password for those seems incorrect.
|
Alternative is we stop detecting when to do this automatically all together and make it a switch in the settings, or we completely remove any of the AiMesh related code. Not sure how much value it provides to people, I don't mind it, I think others like @TheS1R probably don't mind it, but I don't think there's an easy way from the primary router to detect of one of the entries is an AiMesh node or an AP. Luckily the current solution doesn't care and continues to work for the primary even if it can't login to the node, it's not a show stopper, so we just need to make it less visible and less of an annoyance to users. |
Double Whoops. I was right the first time
Ah, so the root cause is that currently we cannot reliably detect whether the main router has an AP or an AiMesh node attached to it based on the NVRAM values being used at this point. This is very unfortunate because it means that the MerlinAU AiMesh menus showing the list of nodes is also unreliable and could be misleading if it's showing an AP that has been misidentified as an AiMesh node. I wonder how the ASUS code correctly detects any APs vs AiMesh nodes (I assume that it does).
I think another annoyance would be to have an AP wrongly listed as an AiMesh node. I don't use AiMesh at all so I can't really check the issues myself, so I'll leave it up to you whether to remove/disable the AiMesh-related menus, or leave the functionality there knowing that it's unreliable and possibly showing misleading information. If it's left "as is" now, I'd suggest letting users know the current limitations and possible "AiMesh mode vs AP" misidentification issues. |
| if [ $? -ne 0 ] | ||
| then | ||
| _UpdateLoginPswdCheckHelper_ FAILURE | ||
| printf "\n${REDct}Login failed for AiMesh Node [$NodeIP_Address].${NOct}\n" |
There was a problem hiding this comment.
I think at least some kind of warning may be useful here just in case it IS an AiMesh node that's having a problem with the login attempt (for some reason).
I would keep it exactly as is, but just make the failures less annoying. |
|
Understanding you don't use aiMesh at all, what about APs? I think one of your cousins or family members had one setup as an AP right? When we did our most recent change regarding APs? Any way that primary router can identify the AP instead of an AiMesh node? Any nvram values maybe? Otherwise we can also make it a switch for on and off in the settings. |
In the string returned by nvram get cfg_device_list, there is a value within delimiters for each node following the MAC address. For mine, it is >1< for the primary router and >0< for each AiMesh node. Likewise, in the string returned by nvram get asus_device_list, there is a value within delimiters for each node following the MAC address. For mine, it is >1< for the primary router and >2< for each AiMesh node. Is it possible that these values are different for an AP? I have another router (RT-AX68U) in the spares bin that I could test with, but that won't happen until at least this afternoon. |
The only reference I have is this from Martinski: #426 (comment) |
|
Actually; thats a lie. I also have this from the user that reported the issue initially: |
Check your forum DM (I didn't want to publicly post MAC addresses). |
So based on your data, which is ALOT of devices, the delimiters of value would be >3< within asus_device_list? |
|
Unless I'm misunderstanding the readings from Martinski, maybe that was taken from the AP itself instead of the parent router? |
No, just the AP. There is a 3 to start next device string, so you are seeing 3 -- look at x in >x<. |
|
Can you run a test for me? Copy and paste this function: And then run it by doing: You should get an IP output of all your nodes, including the AP
Exactly what I am looking at, yes. No confusion here. |
Once you have the output and confirm it includes the AP. Try this function instead: And run it again with: Let me know if it excludes the AP that time! :) |
Ah I understand, you thought because I said lots of devices, I was talking about the 3 at the start of the nodes, but no I meant is that from all devices you have, the >3 at the end is of the test_ap is what we care about to exclude. That means that >2 is from the nodes |
EXACTLY! I just now returned home, and I will run your test shortly. |
Once I had my second coffee and gave it some thought I understood where my communication went wrong. But I think as long as the theory holds that solution should hold. I think my confusion can be ignored since Martinskis results were from the AP itself |
|
Exactly as you predicted, I think — the first run includes primary router, four (4) AiMesh nodes, and the AP. The second run includes ONLY the four (4) AiMesh nodes, removing both the primary router and the AP. TheS1R@routS1R:/tmp/home/root# GetNodeIPv4List()
|
|
Yes, it made it much more obvious when I had both AiMesh nodes AND an AP on the network! |
|
The finale question I have for you is how much value does this feature hold to you? Like... What's the difference between this feature and just installing it on the nodes themselves and letting them handle it all? Well with this feature, you get one email with a summary of all the nodes and their updates. As for without it, each node would send its own email "spamming" your inbox. So the question is, is that worth while maintaining for the value it provides? |
I can go either way on this. I waffle back and forth between running Merlin and stock firmware on the nodes, based upon where the two baselines stand. I don't always have MerlinAU installed on the nodes, which is why the feature is helpful. |
Valid point, it can be a good way to check the firmware version on the nodes without installing MerlinAU, as well as saving the spamming. I think considering we have a better solution (in theory for now) I'll park this idea of removing it and instead work on implementating this new function |
|
Thank you for the data points; this helped us better design a solution to detect the APs over the AiMesh nodes, for now I've opened a new PR and will close this one. |
For AiMesh nodes, the password is always the same as the primary with no options to change, so we don't need the passwordloginchecker helper for those.
And for an AP, they can be set to any password that doesn't match the primary, so displaying invalid password for those seems incorrect.
Addresses this report:
Post in thread 'MerlinAU v1.4.7 - The Ultimate Firmware Auto-Updater (WEBUI + GNUTON SUPPORT!)' https://www.snbforums.com/threads/merlinau-v1-4-7-the-ultimate-firmware-auto-updater-webui-gnuton-support.91326/post-957855