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

Update app name to Trio #157

Merged
merged 13 commits into from
May 9, 2024
Merged

Conversation

marionbarker
Copy link
Contributor

The following steps were used to update the app name to Trio.

  1. rename scheme for phone and watch
  2. rename icons
  3. modify 4 lines in Config.xconfig
  4. inside Xcode, global substitute / replace Trio for Open-iAPS
  5. rename folder open-iaps-oref to trio-ref
  6. update Browser Build files (yml plus fastlane files)
  7. update README.md with correct build information and URL name

Each step used one or two atomic commits to make it easy to follow for reviewers.

Testing:

  • completed: The Mac / Xcode build test was successful on several phone including one with a watch.
  • planned: A browser build test is planned

@marionbarker
Copy link
Contributor Author

It is probably easiest to test using a fresh download to a new folder. Otherwise you may have "debris" left-over from other branches.

If you choose to use the same folder, clean the build folder and close Xcode workspace. Make sure you only have Trio.xcworkspace folder available (no other *.xcworkspace folder should exist.) Then open Xcode again. It is possible you may also need to clean derived data ('rm -rf ~/Library/Developer/Xcode/DerivedData').

@marionbarker
Copy link
Contributor Author

The Browser Build test was successful.

@MikePlante1
Copy link
Contributor

MikePlante1 commented May 7, 2024

I started to build in Xcode but stopped it a few seconds after hitting build/run, realizing I should test in Fastlane first. I checked Apple Developer and the org.nightscout.TEAMID.Trio identifier was created by Xcode but not the other three. I did not delete the alive branch, but I built via Fastlane which was successful.

I did notice icon names are off, so I'll put together a PR for that shortly.

@MikePlante1
Copy link
Contributor

I noticed there are still a lot of iAPS references in localized files. Tomorrow (Tuesday) I'll go through and update those, making sure the original string they're localizing matches.

@MikePlante1
Copy link
Contributor

Localizations has been pushed.

I still see these iaps and Oi references in the repo, though. Do these need to be updated elsewhere as well? Should I go ahead and just rename them all to Trio?

remaining iaps

@bjornoleh
Copy link
Contributor

Localizations has been pushed.

I still see these iaps and Oi references in the repo, though. Do these need to be updated elsewhere as well? Should I go ahead and just rename them all to Trio?

remaining iaps

Perhaps @avouspierre could take a look at this? He is about to refactor something, possibly connected to some of this.

@marionbarker
Copy link
Contributor Author

The ones in main.swift are used for a parser that only @itsmojo uses. I suggest you leave those alone for now. I don’t know about the others.

@itsmojo
Copy link
Contributor

itsmojo commented May 7, 2024

The use of iaps & iAPS in OmniBLE/OmniBLEParser/main.swift applies to iAPS & Trio as well as any other FreeAPS based app with a Loop style pumpManager. Here's my suggested changes to this file to handle the general case correctly and which will avoid using a specific app name like iAPS or Trio.

diff --git a/OmniBLEParser/main.swift b/OmniBLEParser/main.swift
index 63a5ad0..c763f56 100644
--- a/OmniBLEParser/main.swift
+++ b/OmniBLEParser/main.swift
@@ -134,7 +134,7 @@ func parseSimulatorLogLine(_ line: String) {
 
 // 2023-09-02T00:29:04-0700 [DeviceManager] DeviceDataManager.swift - deviceManager(_:logEventForDeviceIdentifier:type:message:completion:) - 563 - DEV: Device message: 17b3931b08030e01008205
 // 2023-09-02T00:29:04-0700 [DeviceManager] DeviceDataManager.swift - deviceManager(_:logEventForDeviceIdentifier:type:message:completion:) - 563 - DEV: Device message: 17b3931b0c0a1d1800b48000000683ff017d
-func parseIAPSLogLine(_ line: String) {
+func parseFreeAPSLogLine(_ line: String) {
     let components = line.components(separatedBy: .whitespaces)
     let hexString = components[components.count - 1]
 
@@ -163,7 +163,7 @@ func parseDashPDMLogLine(_ line: String) {
 func usage() {
     print("Usage: [-q] file...")
     print("Set the Xcode Arguments Passed on Launch using Product->Scheme->Edit Scheme...")
-    print("to specify the full path to Loop Report, Xcode log, RPi pod sim log, iAPS log, or DASH PDM log file(s) to parse.\n")
+    print("to specify the full path to Loop Report, Xcode log, RPi pod sim log, FreeAPS log, or DASH PDM log file(s) to parse.\n")
     exit(1)
 }
 
@@ -207,10 +207,10 @@ for arg in CommandLine.arguments[1...] {
             // 2023-09-02T00:29:04-0700 [DeviceManager] DeviceDataManager.swift - deviceManager(_:logEventForDeviceIdentifier:type:message:completion:) - 563 - DEV: Device message: 17b3931b08030e01008205
             // 2023-09-02T00:29:04-0700 [DeviceManager] DeviceDataManager.swift - deviceManager(_:logEventForDeviceIdentifier:type:message:completion:) - 563 - DEV: Device message: 17b3931b0c0a1d1800b48000000683ff017d
             case Regex("Device message: [0-9a-fA-F]+$"):
-                // Don't mistakenly match an iaps xcode log file line as an iaps log file line
+                // Don't mistakenly match an FreeAPS xcode log file line as an FreeAPS log file line
                 // 2023-10-28 22:37:24.584982-0700 FreeAPS[6030:4151040] [DeviceManager] DeviceDataManager.swift - deviceManager(_:logEventForDeviceIdentifier:type:message:completion:) - 563 DEV: Device message: 17eed3be3824191c494e532e2800069406024c0001f4010268000000060279a404f005021e040300000001ca
                 if !line.contains(" FreeAPS") {
-                    parseIAPSLogLine(line)
+                    parseFreeAPSLogLine(line)
                 }
 
             case Regex("I PodComm pod command: "):

@bjornoleh
Copy link
Contributor

@itsmojo , will you make the change in OmniBLE, so we can update our submodule?

https://github.com/search?q=repo%3ALoopKit%2FOmniBLE%20iAPS&type=code

@MikePlante1
Copy link
Contributor

I don't think OmniBLE is a blocker at all, so if it doesn't get updated before everything else in this PR is approved we can just update the submodule link whenever that happens.

I just amended the Localization commit to include FAX converts.

I have one more commit ready to go for StateiAPSResults and TestFileStorageOiAPS, but would like to add the fix for Info to it before pushing it. Anyone know anything about that one?

After that, I think I can add my ✅

@itsmojo
Copy link
Contributor

itsmojo commented May 7, 2024

The OmniBLEParser/main.swift change to remove iAPS is not a blocker. I will attempt to create a PR for this as I suggested, but it might take me awhile to do this and I don't even know if I have the needed permissions.

@marionbarker
Copy link
Contributor Author

I have permissions to create a branch in Loopkit OmniBLE with changes and can make the PR. Then Trio just needs to update using either the SHA from PR branch or from dev depending on how long it takes to get the PR reviewed. I will DM with @itsmojo to make this happen.

@MikePlante1
Copy link
Contributor

@avouspierre Looks like you added this line. Is it okay to update that to org.nightscout.trio.background-task.critical-event-log or '$(PRODUCT_BUNDLE_IDENTIFIER).background-task.critical-event-log' or something?

Same question about this line you added.

@avouspierre
Copy link
Contributor

avouspierre commented May 9, 2024

@MikePlante1 :

The first line (BGTaskSchedulerPermittedIdentifiers) could be removed because we didn't use a background task with remote notification. I tested (in my current branch) without and seems OK.

CFBundleURLName is useful for garmin app when you link garmin connect to the app. You could change it with dynamic variable. You could use BUNDLE_IDENTIFIER as a URL.

in Shortcuts, HealthKit, FileStorageTests, and URLs
@MikePlante1
Copy link
Contributor

Thanks avouspierre!

With that piece in place, I think we're good to merge this into dev now.

@marionbarker
Copy link
Contributor Author

I guess I can't add myself as a reviewer since I began this PR; if I could, I would approve this PR.

I tested the latest version using Xcode onto a test phone with Trio on it.
Fresh download.
commit: 08e6a80 (HEAD -> trio-rename, origin/trio-rename, origin/HEAD) scrub old name from code

t1dude and others added 2 commits May 9, 2024 15:59
Re-write of the Contribute section.
@marionbarker
Copy link
Contributor Author

After testing, I reviewed the modification to README.md by Magnus and accepted the PR.
There is no need to test again as this is a documentation change only.

@Sjoerd-Bo3
Copy link
Contributor

I'll build now :)

@MikePlante1 MikePlante1 requested a review from t1dude May 9, 2024 14:25
Copy link
Contributor

@t1dude t1dude left a comment

Choose a reason for hiding this comment

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

Built and tested on clean Mac

@MikePlante1 MikePlante1 merged commit 4a75c68 into nightscout:dev May 9, 2024
@MikePlante1 MikePlante1 mentioned this pull request May 10, 2024
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.

7 participants