Fix HomeKitDescribable conformance missing from platform stubs - #25
Merged
Conversation
The #else stub implementations for Accessory, Characteristic, and Service did not conform to HomeKitDescribable or provide the required properties, which would break compilation on non-HomeKit platforms. https://claude.ai/code/session_016TjqsNiRBkhaxyrksWzYeU
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes non-HomeKit (#else) platform stubs so they conform to HomeKitDescribable, preventing compilation failures when HomeKit can’t be imported.
Changes:
- Add
HomeKitDescribableconformance to non-HomeKit stubs forService,Characteristic, andAccessory. - Implement missing
HomeKitDescribablerequirements (localizedDescription/uniqueIdentifier) on those stubs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Sources/HomeAtlas/Service.swift | Makes the non-HomeKit Service stub conform to HomeKitDescribable. |
| Sources/HomeAtlas/Characteristic.swift | Makes the non-HomeKit Characteristic stub conform to HomeKitDescribable and adds uniqueIdentifier. |
| Sources/HomeAtlas/Accessory.swift | Makes the non-HomeKit Accessory stub conform to HomeKitDescribable and adds localizedDescription. |
Comments suppressed due to low confidence (1)
Sources/HomeAtlas/Service.swift:231
uniqueIdentifieris implemented as a computed property returning a newUUID()on every access. That violates the intent ofHomeKitDescribable.uniqueIdentifierbeing a stable instance identifier and can break any client code that keys caches/dictionaries or compares identities. Make this a stored property initialized once per instance (e.g.,public let uniqueIdentifier = UUID()).
public var localizedDescription: String { "" }
public var isPrimaryService: Bool { false }
public var isUserInteractive: Bool { false }
public var uniqueIdentifier: UUID { UUID() }
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+317
to
321
| public final class Accessory: HomeKitDescribable { | ||
| public var name: String { "" } | ||
| public var uniqueIdentifier: UUID { UUID() } | ||
| public var localizedDescription: String { "" } | ||
| public var isReachable: Bool { false } |
| open class Characteristic<Value>: HomeKitDescribable { | ||
| public var characteristicType: String { "" } | ||
| public var localizedDescription: String { "" } | ||
| public var uniqueIdentifier: UUID { UUID() } |
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures the non-HomeKit (#else) platform stubs for core wrapper types compile and match the HomeKitDescribable protocol contract, aligning behavior across HomeKit and non-HomeKit builds.
Changes:
- Make
Service,Characteristic, andAccessorystubs conform toHomeKitDescribable. - Add missing required properties (
localizedDescription/uniqueIdentifier) to the stub types.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Sources/HomeAtlas/Service.swift | Makes non-HomeKit Service stub conform to HomeKitDescribable. |
| Sources/HomeAtlas/Characteristic.swift | Makes non-HomeKit Characteristic stub conform and adds uniqueIdentifier. |
| Sources/HomeAtlas/Accessory.swift | Makes non-HomeKit Accessory stub conform and adds localizedDescription. |
Comments suppressed due to low confidence (1)
Sources/HomeAtlas/Service.swift:231
uniqueIdentifieris defined as a computed property returningUUID()each access. That violates the protocol’s intent of being a stable instance identifier and can break any caller logic that uses it as a key (e.g., dictionaries/sets) because it changes across reads. Consider making it a stored property initialized once (e.g., set ininit, with an optionaluniqueIdentifierparameter for tests).
open class Service: HomeKitDescribable {
public var serviceType: String { "" }
public var name: String? { nil }
public var localizedDescription: String { "" }
public var isPrimaryService: Bool { false }
public var isUserInteractive: Bool { false }
public var uniqueIdentifier: UUID { UUID() }
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+317
to
321
| open class Characteristic<Value>: HomeKitDescribable { | ||
| public var characteristicType: String { "" } | ||
| public var localizedDescription: String { "" } | ||
| public var uniqueIdentifier: UUID { UUID() } | ||
| public var supportsRead: Bool { false } |
Comment on lines
318
to
321
| public var name: String { "" } | ||
| public var uniqueIdentifier: UUID { UUID() } | ||
| public var localizedDescription: String { "" } | ||
| public var isReachable: Bool { false } |
- Fix hashFiles('**/Package.resolved') failure on Linux: Package.resolved
is gitignored, causing the cache step to crash in containers. Use
Package.swift instead, which always exists and drives the dependency graph.
- Rewrite codegen sanity check to avoid pipefail issues: use `shell: bash {0}`
(no implicit set -eo pipefail), capture exit codes explicitly, and write
diff output to a temp file instead of piping through grep chains.
- Fix uniqueIdentifier in platform stubs: change from computed property
returning UUID() (new value each access) to stored `let` property
initialized once per instance, matching protocol semantics.
https://claude.ai/code/session_016TjqsNiRBkhaxyrksWzYeU
Use git diff --name-only + grep instead of pathspec exclusion to avoid potential shell quoting issues. Add git status and diff --stat output to identify exactly which files change after the generator runs. https://claude.ai/code/session_016TjqsNiRBkhaxyrksWzYeU
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes non-HomeKit platform stubs so the package compiles when HomeKit can’t be imported, by aligning stub wrapper types with the HomeKitDescribable protocol and tightening CI’s codegen-diff detection.
Changes:
- Add
HomeKitDescribableconformance and missing required properties to non-HomeKit stubs forAccessory,Service, andCharacteristic. - Update CI cache keys to hash
Package.swift(sincePackage.resolvedis not present in-repo). - Make the codegen “no diffs” CI step more explicit about generator failures and diff reporting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Sources/HomeAtlas/Service.swift | Non-HomeKit stub now conforms to HomeKitDescribable and provides a stable UUID. |
| Sources/HomeAtlas/Characteristic.swift | Non-HomeKit stub now conforms to HomeKitDescribable and provides a stable UUID. |
| Sources/HomeAtlas/Accessory.swift | Non-HomeKit stub now conforms to HomeKitDescribable and provides required properties. |
| .github/workflows/ci.yml | Adjust SPM cache keys and refine codegen sanity checks and diff reporting. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+224
to
+230
| open class Service: HomeKitDescribable { | ||
| public var serviceType: String { "" } | ||
| public var name: String? { nil } | ||
| public var localizedDescription: String { "" } | ||
| public var isPrimaryService: Bool { false } | ||
| public var isUserInteractive: Bool { false } | ||
| public var uniqueIdentifier: UUID { UUID() } | ||
| public let uniqueIdentifier: UUID = UUID() |
| open class Characteristic<Value>: HomeKitDescribable { | ||
| public var characteristicType: String { "" } | ||
| public var localizedDescription: String { "" } | ||
| public let uniqueIdentifier: UUID = UUID() |
Comment on lines
124
to
133
| run: | | ||
| set +e | ||
|
|
||
| # Re-run generator with the current catalog and ensure tree remains clean | ||
| swift run HomeKitServiceGenerator Resources/homekit-services.yaml --output Sources/HomeAtlas/Generated | ||
| # Check for diffs, ignoring timestamp changes (line 2 of generated files) | ||
| if git diff --exit-code -- ':!**/Generated/**/*.swift'; then | ||
| echo "No unexpected changes outside Generated directory" | ||
| else | ||
| echo "ERROR: Unexpected changes detected outside Generated directory" | ||
| GEN_EXIT=$? | ||
| if [ $GEN_EXIT -ne 0 ]; then | ||
| echo "::error::Generator exited with code $GEN_EXIT" | ||
| exit 1 | ||
| fi |
Comment on lines
+142
to
+146
| NON_GEN_DIFF=$(git diff --name-only | grep -v '/Generated/' || true) | ||
| if [ -n "$NON_GEN_DIFF" ]; then | ||
| echo "::error::Unexpected changes detected outside Generated directories:" | ||
| echo "$NON_GEN_DIFF" | ||
| git diff -- $(echo "$NON_GEN_DIFF" | head -20) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The #else stub implementations for Accessory, Characteristic, and Service
did not conform to HomeKitDescribable or provide the required properties,
which would break compilation on non-HomeKit platforms.
https://claude.ai/code/session_016TjqsNiRBkhaxyrksWzYeU