mwarning / trigger

Android app to lock/unlock/ring doors. Supports generic HTTPS/SSH/Bluetooth/MQTT and Nuki Smartlock.
GNU General Public License v3.0
133 stars 22 forks source link

WiFi reconnect detection #54

Closed StickyDigit closed 3 years ago

StickyDigit commented 3 years ago

Testing your good work to fix the four status's at startup thing (which really speeds up startup.. nice!), I observed the following:-

It seems to be detecting WiFi changes, but not actually re-getting the status on reconnect. The behaviour is different with and without fixed SSID. The situation is only resolved by chanigng config, or quitting and re-opening.

Testing first on Lineage 18.1 official with MindTheGapps (android 11). This one has a SIM card.

Settings1: WiFi required on. Fixed SSID set. Location permission granted so the app can see the SSID. Mobile data on as fallback. Turning off WfFi instantly displays "SSID Mismatch (connected to ''"), and the "No connection" image. Re-enabling WiFi, at the moment of reconnect displays "WiFi disabled". Lock/Unlock buttons do not work.

Settings2: Same, but fixed SSID empty. Turning off WiFi instantly displays "The was a problem while connecting to $address::$port" and the "Unknown status" image. Re-enabling WiFi, at the moment of reconnect displays "WiFi disabled", and it switches to the "No connection" image. Lock/Unlock buttons do not work. In this case, the "Unknown status" phase does not happen with mobile data off.

In both cases the app doesn't attempt to recheck the status on reconnect, but notably seems to incorrectly detect the moment of reconnect as "WiFi disabled".

On a LineageOS 14.1 (Android 7) with no mobile data capability, it behaves as above, as if it had mobile data (two states shown in Settings2).

I have only tested by turning wifi off and on again. If you want me to test by turning the A/P off and on, just ask and I'll go sit at a wired computer.

mwarning commented 3 years ago

I cannot reproduce Settings1 and Settings2 on my phone (lineage based on Android 10). But I suspect a timing issue.

Btw.:

StickyDigit commented 3 years ago

I've got a lineage 17 though without a SIM. Will test that too and revise more accurately (and hopefully concisely) tomorrow. Will also check dropping the AP rather than turning WiFi off to test. Hope missus Sticky will forgive the outages.

FWIW, the base of the config was as in previous from me. SSH, simple stop,start,config. Sorry I should have said.

Tapping the background indeed gets me status and stop/start buttons come alive again. Good to know, albeit not obvious to my crumbling old brain :-|

mwarning commented 3 years ago

No need to test AP disconnect.It should be the same as disabling WiFi. Can you try to remove the if statement around this call to callRequestHandler? https://github.com/mwarning/trigger/blob/master/app/src/main/java/app/trigger/MainActivity.java#L270 That might solve the problem.

mwarning commented 3 years ago

@StickyDigit I can send you an apk to install&test, but you have to send me an email first.

StickyDigit commented 3 years ago

Odd.. I responded in this thread from my Studio machine by email after I built and tested it the other day. The message is in my 'sent'. Perhaps github had a funny 5 and sent it to /dev/null.

Removing the 'if' as suggested fixed the basic issue. The app gets sane again on WiFi reconnect and checks status autonomously.

With or without a fixed SSID set it checks status twice on reconnect.

20210612_134629.803345588 : invoked with 'status' 20210612_134629.819850526 : invoked with 'status' 20210612_134629.803345588 : UNLOCKED 20210612_134629.819850526 : UNLOCKED

Unless I turn off mobile data before toggling WiFi.

20210612_134710.538277934 : invoked with 'status' 20210612_134710.538277934 : UNLOCKED

mwarning commented 3 years ago

If it fixes the issue then that is fine with me. Two checks are not that much of an issue, but let's see if we can fix this another way.

mwarning commented 3 years ago

@StickyDigit can you try to remove intentFilter.addAction(ConnectivityManager.CONNECTIVITY_ACTION);? Maybe that fixes the double status request.

StickyDigit commented 3 years ago

Commenting 144 stops Trigger from automatically retrying status on reconnect at all. Restoring the 'if' with that commented still it's still broken.

Undoing that here. Reverting to how I had it on Saturday ('if' removed (268 and 271 commented)). Now it's back to double-'status' if mobile data is on, and once if not.

The following may be a clue, or unrelated. If the latter, and you feel it deserves attention, please paste it into a fresh issue or ask me to.

Once SSID is probed and location perm granted, and even with SSID reset to empty in the only config, it still raises the location status thingy for a moment at every action as if still probing for SSID.

Took a backup of the config, then deleted app data and cache.The location 'ping' does not occur on a fresh config with no SSID set. Setting SSID asks permission to check location (of course) then seems to latch the probe for SSID at every event thereafter.

Starting fresh: Force quit, then clear cache and data, and manually granted location. Launched trigger with no config. It probed location at start.

mwarning commented 3 years ago

@StickyDigit I tried to fix WiFi on/off detection. Please give it try and let me know if that works for you. I have not gotten around to solve the two invocation when WiFi is connected, but it's not that much of a problem I think.

StickyDigit commented 3 years ago

Pulled, built, and installed master. All good. Detects Wi-Fi reconnect just fine. What's more, I only see one status check, with or without mobile net enabled. Nice one! Hitting close.