Experimental support for auto-generated HAP types

[gbrooker]

This PR is experimental, any thoughts or suggestions very welcome. It adds a macOS command line tool hap-config which can be used to generate a complete set of valid HAP constants, data types, characteristics and services from Apple's HomeKit configuration file which has shipped in recent versions of macOS.

The generated file HAP-Services.swift has been added to the HAP package (it's location may need to be changed for anything other than Xcode builds), and original definitions of constants or classes now auto generated have been removed from the codebase.

HAP-Service.swift is huge (over 4000 lines), most of which is defining the Service classes. I debated splitting it out into individual class files, but preferred to keep it monolithic for now.

The Apple HomeKit configuration file contains some enumerations for certain characteristics, but not all. I have hardcoded default enumeration values in HapInspector.swift for many known enumerations not described in the current Apple config file.

I have enclosed all enumerated type definitions in an otherwise empty HAP class, to keep them in a walled off namespace.

Each service has all required Characteristics defined as properties, and optional Characteristics are optional properties. The init method of each Service takes an parameter optionalCharacteristics which is a [CharacteristicType] (defaults to []) containing a list of the optional characteristics to add. Optional Characteristics cannot be added or removed after the Service is created, but the Characteristic values or other properties can be changed later.

Each Service class has the suffix Base in the name. I did this to allow further customisation at the accessory level, setting up parameters, adding delegate methods etc (see Info.swift or Lightbulb.swift). I'm not sure if that is necessary, or if there is a better work than Base. As a consequence in each accessory I have created empty Service classes extending each Base class used, though I'm not really keen on that.

One or two characteristic property names are different in Apples config file, e.g powerState instead of on for the Lightbulb, which will require changes in any clients which use these characteristics.

I have updated the test suite and it appears to confirm to all tests.

The HAP-Services.swift file included in this PR has been generated on macOS 10.14.4 beta 2.

[Bouke]

This looks great! I do have some ideas on the code that's generated, how it interacts with the code-base, separation of packages etc, but I'll need some time to dive into this.

[gbrooker]

Hope you enjoy playing with this, I certainly did ! I tried out a few different ideas for the auto generated code, and got to the stage where I thought it would be best to stop my self re-thinking it over and over and open it up for other ideas. A few principles I held while experimenting were:

• It should be possible for Services to be extended/subclassed in different ways for different Accessories, not hardcoded to include all optional characteristics all the time.
• it should be possible to override the characteristic(didChangeValue:) function in a Service extension/superclass.
• the characteristic properties should be declared with let statements, to avoid them being swizzled after the Service is created.

[Bouke]

I've made an update so it works with SwiftPM, moved the files around so it will now generate a command hap-update, which will generate Sources/HAP/Base/Generated.swift. Also note that I removed Xcode's file headers.

I haven't gotten yet to the actual code that's being generated, will look into that in the next days or so.

[Bouke]

I rebased the PR onto master, which is now based on Swift NIO.

[gbrooker]

Hmm, seems after moving and renaming the file HapInspector.swift, github no longer considers all that work to be my contribution ...

[Bouke]

I really like the idea to have the optional characteristics also available on the services. The API however needs some further thought. For example take the DoorbellBase that defines an optional characteristic volume:

open class DoorbellBase: Service {
    public let programmableSwitchEvent = GenericCharacteristic<UInt8>(...)
    public let brightness: GenericCharacteristic<Int>?
    public let volume: GenericCharacteristic<Int>?
    public init(optionalCharacteristics: [CharacteristicType] = []) { ... }
}

When creating a doorbell with a volume control that goes 0-10, you would to the following:

let doorbell = DoorbellBase(optionalCharacteristics: [.volume])
doorbell.volume!.maxValue = 10

// or subclassing

class MyDoorbell: DoorbellBase {
    init() {
        super.init(optionalCharacteristics: [.volume])
        volume!.maxValue = 10
    }
}

Or similarly when a device has a name:

let speaker = SpeakerBase(optionalCharacteristics: [.name])
speaker.name!.value = "My Speaker"

While it works, I feel we should explore additional syntax to make the API somewhat nicer to work with. For example, (not sure if doable):

let doorbell = DoorbellBase([.volume(maxValue=10)])
let speaker = SpeakerBase([.name("My Speaker")])

Maybe we could make the syntax so easy to work with, we wouldn't even have to create a pair two classes and just drop the Base suffix.

[Bouke]

I've pushed an update to your branch that mostly results in the API outlined above. I've moved instantiation of characteristics to separate methods, so the Services simply forward to those methods. In the process I also made some changes to the output to improve readability (imo).

Due to changes in the API design over time, there's some duplication resulting from AnyCharacteristic / PredefinedCharacteristic. I'm ok with this duplication, as it requires some API redesign before this can be resolved.

I did a test run with the default hap-server, but it appears to be broken now. I get "device unresponsive", but that could be my doing.

[Bouke]

That’s not right, where are you seeing this?

Op 12 feb. 2019 om 21:36 heeft Guy Brooker notifications@github.com het volgende geschreven:

Hmm, seems after moving and renaming the file HapInspector.swift, github no longer considers all that work to be my contribution ...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[gbrooker]

I've made an update so it works with SwiftPM, moved the files around so it will now generate a command hap-update, which will generate Sources/HAP/Base/Generated.swift.

I'd recommend against having it auto-generate on each build, if that is the intention, as it wouldn't work on non-macOS systems, and because the file it is reading is undocumented from a PrivateFramework, it can't be guaranteed to be there forever. It is also possible to have a different HAP spec than the machine on which one is building. The auto-generated definitions I uploaded was created on macOS 10.14.4 beta 2 for instance, including the very latest changes to the new accessories, even though my main build tool chain runs on 10.14.2.

[gbrooker]

I rebased the PR onto master, which is now based on Swift NIO.

I'll have to start trying out the NIO backend, I've been holding off so far. Have you been able to test it running for multiple weeks yet ? Any signs of the slow leaking of ports and memory we had to fix in the previous implementation ?

[gbrooker]

really like the idea to have the optional characteristics also available on the services. The API however needs some further thought. For example take the DoorbellBase that defines an optional characteristic volume:

I had tried this line of thought out myself, but discarded it. I didn't like the extra parenthesis you end up with on all Characteristic types you are not specifying arguments for. I personally much prefer the former for readability:

let doorbell = DoorbellBase(optionalCharacteristics: [.volume])
doorbell.volume?.maxValue = 10

let speaker = SpeakerBase(optionalCharacteristics: [.name, .volumeControlType, .volumeSelector, .volume])
speaker.name?.value = "My Speaker"'''

than

let doorbell = DoorbellBase(optionalCharacteristics: [.volume(maxValue=10)])

let speaker = SpeakerBase([.name("My Speaker"), .volumeControlType(), .volumeSelector(), .volume()])

In the vast majority of cases, the system defined max and min values will not be changed. colorTemperature in Lightbulb is a one of very few examples.

There will be many cases that an accessory will want to set the initial value of a characteristic, however I don't think that should be done in the Service.init statement. It will still need to be done in subsequent statements for required characteristics, so why do it differently for optional characteristics ?

I also think it is clearer to keep the parameter label optionalCharacteristics: in the Service init(). We could shorten it to optional if you think it is too long, but removing it loses the clarity that the arguments are specifying what is optional, and should not include any required characteristics.

[gbrooker]

Maybe we could make the syntax so easy to work with, we wouldn't even have to create a pair two classes and just drop the Base suffix.

I agree that it would be preferable to drop the Base suffix. I did it for compatibility reasons, but having been through all the accessories, and in most cases discovered that there was nothing to add, I think the Base suffix is not required. If the Service needs superclassing, it can use a custom name in the corresponding Accessory.

[gbrooker]

Isn't Enums a little too generic for the wrapper namespace around CharacteristicType's ? You didn;t like HAP ?

[gbrooker]

That’s not right, where are you seeing this?

Yes, github treats the move and rename as a deletion of my file, and a new addition authored by you. Oh well, sigh....

[Bouke]

Yes, github treats the move and rename as a deletion of my file, and a new addition authored by you. Oh well, sigh....

I've squashed my file move with your commit. As a result, there's no longer a file move. I added your name to the changelog to give you credits for this nice contribution. The git history will continue to show your initial version, as time moves on and further refactors will happen.

Isn't Enums a little too generic for the wrapper namespace around CharacteristicType's ? You didn;t like HAP ?

Enums might be a little bit too generic, maybe HAPEnums instead? HAP would also cause confusion as the module is already named HAP.

import HAP
// probably both valid:
HAP.HAP.CurrentAirQuality
HAP.CurrentAirQuality

There will be many cases that an accessory will want to set the initial value of a characteristic, however I don't think that should be done in the Service.init statement. It will still need to be done in subsequent statements for required characteristics, so why do it differently for optional characteristics ?

I made further changes to the initialization of the services. It now allows you to specify both required and optional characteristics. It will create the required characteristics if you haven't specified them on init. I feel it makes for a clean API:

let leftSpeaker = SpeakerBase(characteristics: [.name("Left"), .volume(5, maxValue: 10)])

// or subclassing:

class MySpeaker: SpeakerBase {
    init() {
        super.init(characteristics: [.name("Left"), .volume(5, maxValue: 10), .mute(True)])
    }
}

I'd recommend against having it auto-generate on each build, if that is the intention(...)

That's not the case. It's just a command that you'd run whenever you feel like it. Ideally this command would be executed by a maintainer whenever there's a macOS update.

I'll have to start trying out the NIO backend, I've been holding off so far. Have you been able to test it running for multiple weeks yet ? Any signs of the slow leaking of ports and memory we had to fix in the previous implementation ?

I have been running it for 2 months now on a Raspberry Pi, no issues so far. I haven't checked for memory / port leakage.

[gbrooker]

I've squashed my file move with your commit. As a result, there's no longer a file move. I added your name to the changelog to give you credits for this nice contribution. The git history will continue to show your initial version, as time moves on and further refactors will happen.

Wow, thank you !

[gbrooker]

Enums might be a little bit too generic, maybe HAPEnums instead? HAP would also cause confusion as the module is already named HAP.

What about just making the namespace Characteristic instead of HAPEnums?

[gbrooker]

I made further changes to the initialization of the services. It now allows you to specify both required and optional characteristics. It will create the required characteristics if you haven't specified them on init. I feel it makes for a clean API:

Good idea

[Bouke]

🎉