mooshim / Mooshimeter-AndroidApp

50 stars 28 forks source link

Does this App has a phone-home functionality? #61

Closed famo closed 7 years ago

famo commented 7 years ago

See: https://en.wikipedia.org/wiki/Phoning_home

Since https://moosh.im/ is down, I have MASSIVE problems connecting and working with my mooshimeter. WTF is going on?

kc0tfb commented 7 years ago

Around line 175 of OADActivity, the app calls Util.getLatestFirmware(); I suspect that may be the "phone home" of which you speak - checking to see if there's a firmware update available. It also uses Crashlytics, but I'll let someone more knowledgeable speak to whether that does network I/O or not.

eldstal commented 7 years ago

I can confirm that the app was nearly unusable yesterday (I was considering selling my mooshimeter, it's not acceptable to have to fiddle with it for 20 minutes every time I want to use it!). Now that the site is back up, it properly connects to the meter again.

famo commented 7 years ago

Yeah, mine is working fine today as well.

@jwhong you really need to fix this!

eldstal commented 7 years ago

Suggested fix is in the attached pull request, with a hard-coded 5 second timeout for connection and read.

The error does not appear if connections to moosh.im are dropped entirely, but only when there is an added delay. This can be verified by using for example Fiddler, and setting up an Autoresponder for connections to http://moosh.im:443 with an action of *delay:50000

famo commented 7 years ago

Actually, it would be even nicer if the fw-checking ("phone home" function) could be moved all together out of the connection part and to the dedicated fw-upload section.

I don't see why it needs to check for a new FW on every new connection to my mooshimeter...

eldstal commented 7 years ago

Yes.

As far as I can tell, no caching of the firmware file is performed, nor is the server version checked against the bundled version before download. This means the app will download the same file over and over again, which seems pretty wasteful. Maybe make the online firmware check/update entirely manual/optional unless required due to a breaking app update?

famo commented 7 years ago

... Maybe make the online firmware check/update entirely manual/optional unless required due to a breaking app update?

Yes, this is what I meant.

If you are connected to your device and go Settings, there is "Start Firmware Uploader" commented with "Checks if new firmware is available for this meter". So only if someone clicks "Go" here, it should check for new firmware.

eldstal commented 7 years ago

On a slightly related note, I'm having trouble getting to moosh.im right now (on a university network in Sweden). I get NXDOMAIN (domain not found), and https://mxtoolbox.com/domain/moosh.im/ also shows a similar error. I don't have my meter handy to check if the app misbehaves.

I'll sit down tomorrow and cut the firmware download out of the main menu. It's James' call if he wants to include this change in the main distribution of the app, but if server problems twice in one week can cause meters to become inoperable it obviously needs to be changed.

eldstal commented 7 years ago

I've updated the pull request, to behave as above. No download is attempted until the user brings up the update dialog, and the firmware is only downloaded once per session. The 5-second timeout is still in place, but could probably be extended now that it isn't holding up normal operation.

There probably should be some reworking of where the downloaded file is stored, so that future checks for auto-update can use the already downloaded firmware file rather than the (older) bundled one.

eldstal commented 7 years ago

The changes have been merged into the master branch (Thanks, James!). If @famo is satisfied, I think the issue can be closed.

famo commented 7 years ago

@eldstal Agreed and thanks a lot for the fast patch! (Could have closed it himself tho.)

joebowbeer commented 6 years ago

Still happening?

https://moosh.im/f/topic/cant-get-out-of-bootloader-mode/ https://moosh.im/f/topic/constantly-updating/

eldstal commented 6 years ago

I'm unable to reproduce (my "No thanks, no update this time" button works as intended).

In any case, this isn't related to the download of firmware file, but rather a separate issue with the update logic. It should probably have its own issue.

Is there anyone with a dev environment who can reproduce this issue?