Official-Kodi-Remote-iOS
Official-Kodi-Remote-iOS copied to clipboard
xbmc
Kodi Remote Watchapp
I've been missing watchOS support and finally decided to implement it. It's written in swift and depend only on APIKit (https://github.com/ishkawa/APIKit). It can send rpc requests directly from watches or via iOS app.
The changes are more or less isolated. Folders "watchapp" and "watchapp Extensions" contain code running on watches. The code in "KodiAPI" is called from watchOS and iOS. Important changes are in "XBMC Remote/AppDelegate.m".
- Before trying, run
carthage updateto setup dependencies.
I would be glad to see the watch support in the Official-Kodi-Remote-iOS app. If you decide to reuse the code you can just take it or; It will be my pleasure to make changes required to merge the code after the review.
Anyway let me know what do you think.
You are simply amazing :) Thank you for your work!
I only ask you to have a little patience because in this moment I'm total full with work damn stuff :(
Again, well done at first glance!
@integralpro Can you setup a TestFlight so that volunteers can test it? Please join: https://forum.kodi.tv/showthread.php?tid=335853
Sure, I can try to do something next Monday. —Pavel
On Sep 13, 2019, at 12:44 AM, Pieter Janssens [email protected] wrote:
@integralpro https://github.com/integralpro Can you setup a TestFlight so that volunteers can test it? Please join: https://forum.kodi.tv/showthread.php?tid=335853 https://forum.kodi.tv/showthread.php?tid=335853 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xbmc/Official-Kodi-Remote-iOS/pull/91?email_source=notifications&email_token=AAJW2OUH5KEDSIMISXAS2NLQJNAFJA5CNFSM4FVWCDH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6UH2IA#issuecomment-531135776, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJW2OQGJMLDD26LUXLQ2BTQJNAFJANCNFSM4FVWCDHQ.
@integralpro are you still interested in the PR?
I haven't reviewed code yet, just a question out of interest: why did you decide to choose APIKit over Alamofire? :)
@kambala-decapitator yes, it would be great if there will be some watch support. I still can work on that. APIKit was just the first thing I found.
Please resolve conflicts then and test your changes with Xcode 12
Yes, I will rebase and cleanup the code.
On Jan 5, 2021, at 11:00 AM, Andrey Filipenkov [email protected] wrote:
Please resolve conflicts then and test your changes with Xcode 12
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xbmc/Official-Kodi-Remote-iOS/pull/91#issuecomment-754833786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJW2OV7ODN7IDQCV4B3GKTSYNOUDANCNFSM4FVWCDHQ.
thanks for updating! Will check in the next days. Please be aware that it won't make it into next update as our primary goal is to release a fixed version that works flawlessly on modern iOS.
This branch has conflicts that must be resolved
;)
This branch has conflicts that must be resolved
;)
They will appear every time project.pbxproj is updated. Let me know when you finish the work related to a next release, I will fix any infrastructure conflicts.
They will appear every time project.pbxproj is updated.
Well, that's not entirely true. It depends on the changes.
IMO build settings in master are correct now, nothing else to update. But It's a good idea to rebase after the release indeed.
I think the issue is with your commit https://github.com/xbmc/Official-Kodi-Remote-iOS/pull/91/commits/08d8ca1ef9eed82fe598ceeb3351b2a797242b30 as I pushed the same change today.
TODO besides fixing conflicts and review comments:
- make the project buildable:
1.1. move
MARKETING_VERSIONfrom target to project level 1.2. ensure that watch app/extension receive the sameCFBundleVersionas the host app (CURRENT_PROJECT_VERSIONshould be dropped) - change bundle ID of new frameworks to match host app
- you could introduce a special "bundle ID prefix" variable to reduce copy-paste, would also be easier to create separate app for debugging purposes
- what about using App Groups to share settings? (might be an overkill just for sharing hosts though)
- You're essentially building the new framework twice. Isn't it possible to build once and share it between apps? (hm, I guess not, as deployment target is different - iOS vs watchOS)
- you could also drop those frameworks and link code directly to targets
- raise deployment target to 4.3, set all deployment targets on project level instead of target level
- add shared scheme for watch extension
- Assets for extension are empty and should be removed
- Why don't you use Assets to bundle watch app's resources?
- if possible, please split your changes into multiple commits: e.g., first commit could be SPM+iOS framework, second would be watch app itself and the third for fixing point 1. The more granular the better :)
- use the latest APIKit
Made runtime test on watch 1 with OS 4.3.2: looks good.
One question... I'm not sure if it's critical is to have CURRENT_PROJECT_VERSION undefined everywhere.
Please see that the current change defines CURRENT_PROJECT_VERSION to 1 (same value as it was in .info for CFBundleVersion for the host), and all .info files now use $(CURRENT_PROJECT_VERSION) it for CFBundleVersion
One question... I'm not sure if it's critical is to have CURRENT_PROJECT_VERSION undefined everywhere.
Please see that the current change defines CURRENT_PROJECT_VERSION to 1 (same value as it was in .info for CFBundleVersion for the host), and all .info files now use $(CURRENT_PROJECT_VERSION) it for CFBundleVersion
currently bundle version of the host app is derived as count of git commits, you can find it in a script build phase. Either simply copy this phase to other targets or pre-compute the value somehow.
One question... I'm not sure if it's critical is to have CURRENT_PROJECT_VERSION undefined everywhere. Please see that the current change defines CURRENT_PROJECT_VERSION to 1 (same value as it was in .info for CFBundleVersion for the host), and all .info files now use $(CURRENT_PROJECT_VERSION) it for CFBundleVersion
currently bundle version of the host app is derived as count of git commits, you can find it in a script build phase. Either simply copy this phase to other targets or pre-compute the value somehow.
Okay, I see. I will do the change.
TODO besides fixing conflicts and review comments:
- make the project buildable: 1.1. move
MARKETING_VERSIONfrom target to project level
done
1.2. ensure that watch app/extension receive the same
CFBundleVersionas the host app (CURRENT_PROJECT_VERSIONshould be dropped)
done
change bundle ID of new frameworks to match host app
- you could introduce a special "bundle ID prefix" variable to reduce copy-paste, would also be easier to create separate app for debugging purposes
dropped frameworks, added .swift files to the corresponding targets
- what about using App Groups to share settings? (might be an overkill just for sharing hosts though)
if needed, may be done in a separate change
You're essentially building the new framework twice. Isn't it possible to build once and share it between apps? (hm, I guess not, as deployment target is different - iOS vs watchOS)
- you could also drop those frameworks and link code directly to targets
Correct, they need to be built twice - per platform. I followed your suggestion and dropped framework targets.
- raise deployment target to 4.3, set all deployment targets on project level instead of target level
done
- add shared scheme for watch extension
done
- Assets for extension are empty and should be removed
done
- Why don't you use Assets to bundle watch app's resources?
done + added missing icons. I remember they are anyway required to for the AppStore.
- if possible, please split your changes into multiple commits: e.g., first commit could be SPM+iOS framework, second would be watch app itself and the third for fixing point 1. The more granular the better :)
Give me a good reason to do that. Don't you plan to squash everything before the merge?
- use the latest APIKit
APIKit is set to 5.1.0, but there is 5.2.0 available. I expected SPM to automatically track it up to a next major release. Let me check that.
One question... I'm not sure if it's critical is to have CURRENT_PROJECT_VERSION undefined everywhere.
Please see that the current change defines CURRENT_PROJECT_VERSION to 1 (same value as it was in .info for CFBundleVersion for the host), and all .info files now use $(CURRENT_PROJECT_VERSION) it for CFBundleVersion
you know, some time after that I've actually realised how to
pre-compute the value somehow
and use the approach you described :)
In my local changes that I'll PR soon I've done the same (since debug builds don't care about the actual build version and 1 is fine) and offloaded CURRENT_PROJECT_VERSION computation to the Fastfile to run only when uploading a build. #235
Sorry, but you'll have to remove script phase changes from 4d75b18e45474c4d6a9c24256fdcedc783ac84f5.
if needed, may be done in a separate change
I think not.
Give me a good reason to do that. Don't you plan to squash everything before the merge?
I prefer clear history with detailed changes. This also follows approach used in contributions to https://github.com/xbmc/xbmc. But if it's cumbersome for this PR (lots of pbxproj changes, I know), then I can squash.
I expected SPM to automatically track it up to a next major release
I've never worked with SPM, but that would've been a bad approach if it had updated dependencies for you implicitly :) That's why update command exists: pod update, carthage update, bundle update to name a few.
Will review your changes soon.
Sorry, but you'll have to remove script phase changes from 4d75b18.
That's okay, I can remove them.
Hi, just compiled this PR and tested the App on the iPhone Xs simulator. It seems the Launch Screen has gone. Instead there is a black screen while loading. Can you take a look at this?
Hi, just compiled this PR and tested the App on the iPhone Xs simulator. It seems the Launch Screen has gone. Instead there is a black screen while loading. Can you take a look at this?
Thanks for checking and letting me know! I will take a look.
@wutschel I accidentally removed launch screen from project.pbxproj. Should be fine now.
@wutschel I accidentally removed launch screen from project.pbxproj. Should be fine now.
Thanks, I confirm it´s back now.
Another question: When building the App for iPhone I get several hundred new warnings caused by changing CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF to YES. Also other new warnings are coming up now. Was this done intentionally?
CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF is a switch to enable compiler warnings for implicit self retains. It was turned on unintentionally, may be at some point I agreed to update project with "recommended settings". Let me check if I can turn this option OFF.
I disabled CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF to suppress the warnings, but let @kambala-decapitator to decide if those retains are the real issue.
There are two new warnings coming with the PR:
- Unused return value in AppDelegate, calling KodiAPI -- I fixed it.
- 'IPHONEOS_DEPLOYMENT_TARGET' is set to 8.0, but the range of supported deployment target versions is ...
The second is because of APIKit dependency, it sets IPHONEOS_DEPLOYMENT_TARGET to 8.0 in the config files. It shouldn't be a problem, however I don't know if it's possible to change it to 9.0 or how to suppress it.
I disabled CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF to suppress the warnings, but let @kambala-decapitator to decide if those retains are the real issue.
it's difficult to say without actually checking every warning. Anyway, it's a topic for another PR.
The second is because of APIKit dependency, it sets IPHONEOS_DEPLOYMENT_TARGET to 8.0 in the config files. It shouldn't be a problem, however I don't know if it's possible to change it to 9.0 or how to suppress it.
this can be ignored. I know how to fix such with cocoapods, but SPM probably doesn't allow that many customisations. If really needed, it can always be solved in a fork of APIKit.
every time I open project with Xcode 12.4, some files become changed, please fix.
❯ git --no-pager diff
diff --git a/Kodi Remote.xcodeproj/project.xcworkspace/contents.xcworkspacedata b/Kodi Remote.xcodeproj/project.xcworkspace/contents.xcworkspacedata
index 3af3be2..919434a 100644
--- a/Kodi Remote.xcodeproj/project.xcworkspace/contents.xcworkspacedata
+++ b/Kodi Remote.xcodeproj/project.xcworkspace/contents.xcworkspacedata
@@ -2,6 +2,6 @@
<Workspace
version = "1.0">
<FileRef
- location = "self:Kodi Remote.xcodeproj">
+ location = "self:">
</FileRef>
</Workspace>
diff --git a/Kodi Remote.xcodeproj/xcshareddata/xcschemes/watchapp.xcscheme b/Kodi Remote.xcodeproj/xcshareddata/xcschemes/watchapp.xcscheme
index 94ad1fb..ee477ec 100644
--- a/Kodi Remote.xcodeproj/xcshareddata/xcschemes/watchapp.xcscheme
+++ b/Kodi Remote.xcodeproj/xcshareddata/xcschemes/watchapp.xcscheme
@@ -63,8 +63,10 @@
debugDocumentVersioning = "YES"
debugServiceExtension = "internal"
allowLocationSimulation = "YES">
- <BuildableProductRunnable
- runnableDebuggingMode = "0">
+ <RemoteRunnable
+ runnableDebuggingMode = "2"
+ BundleIdentifier = "com.apple.Carousel"
+ RemotePath = "/Kodi Remote">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "92663CA7203021D5002FE988"
@@ -72,7 +74,7 @@
BlueprintName = "watchapp"
ReferencedContainer = "container:Kodi Remote.xcodeproj">
</BuildableReference>
- </BuildableProductRunnable>
+ </RemoteRunnable>
</LaunchAction>
<ProfileAction
buildConfiguration = "Release"
@@ -80,8 +82,10 @@
savedToolIdentifier = ""
useCustomWorkingDirectory = "NO"
debugDocumentVersioning = "YES">
- <BuildableProductRunnable
- runnableDebuggingMode = "0">
+ <RemoteRunnable
+ runnableDebuggingMode = "2"
+ BundleIdentifier = "com.apple.Carousel"
+ RemotePath = "/Kodi Remote">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "92663CA7203021D5002FE988"
@@ -89,7 +93,16 @@
BlueprintName = "watchapp"
ReferencedContainer = "container:Kodi Remote.xcodeproj">
</BuildableReference>
- </BuildableProductRunnable>
+ </RemoteRunnable>
+ <MacroExpansion>
+ <BuildableReference
+ BuildableIdentifier = "primary"
+ BlueprintIdentifier = "92663CA7203021D5002FE988"
+ BuildableName = "watchapp.app"
+ BlueprintName = "watchapp"
+ ReferencedContainer = "container:Kodi Remote.xcodeproj">
+ </BuildableReference>
+ </MacroExpansion>
</ProfileAction>
<AnalyzeAction
buildConfiguration = "Debug">
You may change those after we merge the PR. I don't have such changes and also use Xcode 12.4.