Bouke / HAP

Swift implementation of the Homekit Accessory Protocol
https://boukehaarsma.nl/HAP/
MIT License
366 stars 50 forks source link

Auto Setup #44

Closed gbrooker closed 6 years ago

gbrooker commented 6 years ago

These changes add the capability to automatically pair from the iOS Home app or Camera app. The setupCode and a new setupKey are moved to the persistent Configuration, for preservation over restarts. A setup hash is computed and added to the configuration record broadcast over MDNS.

HomeKit matches the setup hash when performing an auto setup. A QR code should be displayed to the user representing a X-HM URI, which is an encoding of the setupCode, the primary accessory type and the setupKey. A new class is added which will create an NSImage from a string

QRcode(device.setupURI).asNSImage

The NSImage can then be added to an NSImageView. The QR Image generation is only available on macOS. Functions providing alternative representations such as an ASCII QR code could be added for other platforms at a later date by other authors.

When automatically pairing, the setupCode is not entered manually, so it does not need to be easily memorable, so can be auto generated (and preserved in the configuration). I have therefore removed it as a required parameter to Device.init().

I have added an optional defaultSetupCode parameter to allow a caller to provide a preferred default setupCode if they really wanted to, instead of relying on an auto generated one. It will be ignored if a setupCode is found in the configuration. I think this is a kludge, and so I thought I'd discuss it with you before firming it up. I've not modified the test cases yet for that reason. I will update the test cases once we've agreed on the best method of handling a user specified setupCode.

I just noticed that I left a few print() statements in Device.setupURI, I'll remove them in my next commit, once you've had a chance to review and try it out.

gbrooker commented 6 years ago

One other note, the pairing identifier must be uppercase for this to work, this is fixed in the commit.

gbrooker commented 6 years ago

A QR code as a String implementation could be inspired by :
https://github.com/gtanner/qrcode-terminal

gbrooker commented 6 years ago

or http://www.linux-magazine.com/Online/Features/Generating-QR-Codes-in-Linux

Bouke commented 6 years ago

Just tested this myself, I must admit that this is REALLY COOL. I generated the QR code using some online tool. It would be very nice to generate the QR code in the terminal, but we'd need some ascii QR generated then. I verified using this QR barcode that even scanning the barcode from Xcode terminal (white-on-black) works as expected!

I found this tutorial on generating QR codes. At the moment that feels like overkill. Maybe we should just include a URL to some online generator and pass the QR code along with it. Similar to this link.

Example;

Scan the following QR code using your iPhone:
http://chart.apis.google.com/chart?cht=qr&chs=200x200&chld=L|0&chl=[url-encoded QR code here]
gbrooker commented 6 years ago

You could do, or just print the X-HM: URI to the terminal, and allow he operatorto paste that text into any online QR code generator. I use HAP from an app, so I display a QR code in an NSImageView.

Alternatively you could port the javascript terminal QR code generator on github into Swift and add it to the QRCode class in a function 'asString"

gbrooker commented 6 years ago

I've added some commits to address your comments. If the Device.init() api is OK like this, I'll update the test cases

gbrooker commented 6 years ago

I've addressed the comments above, fixed swiftlint pedantries and verified HAP suite tests. The travis tests failed due to arc4random_uniform not being available on that platform. I have provided an implementation which passes the travis tests, however I have no way to test it. Appreciate if you could verify that it functions correctly.

Bouke commented 6 years ago

The travis tests failed due to arc4random_uniform not being available on that platform. I have provided an implementation which passes the travis tests, however I have no way to test it. Appreciate if you could verify that it functions correctly.

There's Cryptor.Random.generateBytes which we probably should use instead.

Bouke commented 6 years ago

See my two commits to the branch and let me know what you think.

Bouke commented 6 years ago

Furthermore I think it'll be good to merge!

gbrooker commented 6 years ago

I really love the ExpressibleByStringLiteral, that is so elegant and impressive. I never knew that protocol existed. I've learnt something today, thanks.

However, I really don't like the changes to random number. Why replace arc4random_uniform on macOS ? Posts I have read say that function will be implemented in Foundation on linux eventually, so surely it is the right method to use, replaced by a temporary workaround on that platform for now. Why replace the recursion with a loop ? Why loop for 100 times ? The number is quite arbitrary, and the function would return an invalid code if it ever got past 100 tries. I much prefer my version. How about a compromise using Random.generate(byteCount) on linux only ?

Bouke commented 6 years ago

I really love the ExpressibleByStringLiteral, that is so elegant and impressive. I never knew that protocol existed. I've learnt something today, thanks.

Yeah it's really nice. Don't overuse it, however, as it might result in the compiler having to resolve multiple branches.

However, I really don't like the changes to random number. Why replace arc4random_uniform on macOS ? Posts I have read say that function will be implemented in Foundation on linux eventually, so surely it is the right method to use, replaced by a temporary workaround on that platform for now. How about a compromise using Random.generate(byteCount) on linux only ?

Fair enough. I'll write up a different implementation.

Why replace the recursion with a loop ? Why loop for 100 times ? The number is quite arbitrary, and the function would return an invalid code if it ever got past 100 tries. I much prefer my version.

The function would not return an invalid code if it doesn't find a valid setup code, but traps instead. I replaced the recursion to put an upper limit on the amount of tries that we do. I haven't calculated the chance, but I think we can safely assume that we should find a setup code in 100 tries. If we can't find a setup code in 100 tries, it's more probable that we have an issue with the random number generator, which would otherwise result in indefinite recursion.

Bouke commented 6 years ago

I've written a new implementation of arc4random_uniform for Linux and guarded against modulo bias and also replaced the loop for an indefinite loop. What do you think of it?

Bouke commented 6 years ago

I've added a QRCode generator library and a transformation to ASCII. It displays nicely within the Xcode debug console, however the boxes show incorrectly in iTerm however.

gbrooker commented 6 years ago

Great additions, you've been busy yesterday ! I struggled at first incorporating the CQRCode framework into my app, Xcode kept giving a build error about a missing module, even though the framework was in the LinkedFrameworks list. I finally managed to fix that by adding a line to Header Search Path in build setting with $(SRCROOT)/../HAP/HAP.xcodeproj/GeneratedModuleMap/CQRCode but I wonder if there is a better way. Unless I'm doing something wrong, perhaps it would be useful for people to include a mention in the documentation.

I made one small change. Thanks for spotting the need to pad the setupKey, but I moved that to the generateSetupKey() function, so it is corrected at source.

I added a couple of other QR representations, which may work better in some terminals. asBigText will use two blocks per pixel, and asAscii is similar but uses two # characters if only a very basic character set is available. These both work for me, though the ascii representation needed me to step back quite a bit from the screen. I tried both using only one ascii character per pixel, but as characters as not square, it resulted in a tall thin QR code that I could only get recognised when holding the phone at acute angles from the screen.

gbrooker commented 6 years ago

Thanks for the changes to arc4random, I think the result is much better.

Bouke commented 6 years ago

Great additions, you've been busy yesterday !

Thanks!

I struggled at first incorporating the CQRCode framework into my app, Xcode kept giving a build error about a missing module, even though the framework was in the LinkedFrameworks list. I finally managed to fix that by adding a line to Header Search Path in build setting with $(SRCROOT)/../HAP/HAP.xcodeproj/GeneratedModuleMap/CQRCode but I wonder if there is a better way. Unless I'm doing something wrong, perhaps it would be useful for people to include a mention in the documentation.

Using SwiftPM you can do swift package generate-xcodeproj and that'll setup the header search path correctly. In your case you're doing a Cocoa-app, I'm not quite sure how that would nicely fit in here -- are you using a workspace?

I tried both using only one ascii character per pixel, but as characters as not square, it resulted in a tall thin QR code that I could only get recognised when holding the phone at acute angles from the screen.

Yeah I ran into the same problem. That's why I combined two lines into a single character in the first place.

By the way; your git committer name/email is off due to the curly quotes. You might want to verify that you've set them correctly: git config --global user.name and git config --global user.email.

I've merged the PR, great work on this nice addition!