Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement auto update in Fieldworks Lite #1298

Merged
merged 19 commits into from
Dec 9, 2024
Merged

Conversation

hahn-kev
Copy link
Collaborator

@hahn-kev hahn-kev commented Dec 2, 2024

Update steps:

  1. check settings for the last time we checked for updates, don't do anything if it was within the last day, can be forced via an env variable
  2. fetch the latest release info from lexbox by calling https://lexbox.org/api/fwlite-release/latest, can be overridden with an env variable
  3. compare the version installed with the latest version using string sorting, if the latest version preceeds the current then we update
  4. give the release download url to the MSIX packing code and tell it to install that version, it will be downloaded and installed once the user closes fw lite

I'm worried about fw lite failing to update itself, if that happens then it makes it hard to recover and the users will need to manually update, I want to avoid that at all costs.
Some failure modes:

  • current version is always considered more recent than the latest version
  • never checks for updates because the date stored in settings is far in the future
  • unable to access API
  • unable to download/apply update

We can avoid some of these but not all, I would like to get input from other devs on this.

  • the app version will be sent to lexbox, so it can sniff the version and return a custom response to workaround a bug in specific version
  • we can validate the last time we checked for an update to avoid never checking because the date is too far in the future

backend/FwLite/FwLiteDesktop/AppUpdateService.cs Outdated Show resolved Hide resolved
.github/workflows/fw-lite.yaml Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Dec 2, 2024

C# Unit Tests

103 tests  +5   103 ✅ +5   5s ⏱️ ±0s
 16 suites +1     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit da8ba2c. ± Comparison against base commit 62e2c19.

♻️ This comment has been updated with latest results.

myieye
myieye previously approved these changes Dec 2, 2024
Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks slick to me.

backend/FwLite/FwLiteDesktop/AppUpdateService.cs Outdated Show resolved Hide resolved
backend/FwLite/FwLiteDesktop/AppUpdateService.cs Outdated Show resolved Hide resolved
backend/FwLite/FwLiteDesktop/AppUpdateService.cs Outdated Show resolved Hide resolved
backend/LexBoxApi/Controllers/FwLiteReleaseController.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case mistakes in "FieldWorks Lite" are the only things that I think clearly need a change.
Everything else is more ideas/thoughts.

backend/LexBoxApi/Controllers/FwLiteReleaseController.cs Outdated Show resolved Hide resolved
backend/FwLite/FwLiteDesktop/App.xaml.cs Outdated Show resolved Hide resolved

public static bool ShouldUpdateToRelease(string appVersion, string latestVersion)
{
return String.Compare(latestVersion, appVersion, StringComparison.Ordinal) > 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory this could simlpy be appVersion != latestVersion.
That would solve are commit-hash problem, but it makes it a lot more sensitive.
In that case, with a bit of parsing we could do a check of this form:
latest.date > app.date || latest.date == app.date && latest.hash != app.hash

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I agree, there's a bunch of different ways we could do it, we could always chose one, but report via otel all the different ways we consider doing it, then we can see over time how they work?

preferences.Set(LastUpdateCheck, DateTime.UtcNow);
return true;
}
if (timeSinceLastCheck.Hours < 20) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine merging this, but I think it deserves more thought.

Checking for updates is cheap.
Installing updates is not.

So, we're not really protecting from checking for updates, but maybe repeated failed updates?

If we release two updates within 24hrs then we probably have good reason to.
Maybe the logic should be more like:

  • Always look for updates on start
  • Don't try the same update more than once within 24hrs (unless it clearly failed due to connectivity problems?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right is is cheap to check. At this point I have no idea what a failed update would look like or how that would even happen considering we're not running any custom code during install. Not that I doubt it could happen. That said, tracking update attempts introduces complexity which I'd rather avoid, so maybe let's leave it as is for now and see what we learn with people actually using it and updating to new versions

@hahn-kev hahn-kev mentioned this pull request Nov 29, 2024
33 tasks
@hahn-kev hahn-kev merged commit ce28810 into develop Dec 9, 2024
16 checks passed
@hahn-kev hahn-kev deleted the chore/auto-update-fw-lite branch December 9, 2024 06:00
@hahn-kev hahn-kev linked an issue Dec 11, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto update FW Lite
2 participants