NFC Lock Chars & Services and WR functionality #801
Conversation
|
This looks like a really cool project - thanks for the PR. I've reviewed the changes as well as the info on all the links referencing HomeKey that you included. A few initial thoughts:
One limitation to note is that HomeSpan cannot be powered by battery (well, maybe a car battery 😄) since it requires an always-on WiFi connection, which is incredibly power-hungry. I believe all commercial lock mechanisms connect to HomeKit via HAP's (low energy) Bluetooth Protocol, which I've not implemented in HomeSpan (it's completely different than the HAP WiFi protocol). Hopefully this limitation does not put a damper on the project. |
|
Thank you for your response. Some comments on your feedback:
I really never intended to integrate the whole thing with HomeSpan since it make no sense given the fact that they serve different purposes and it's also a lot of code to add for no reason just to burden the whole thing. NFC would've been nice i guess but that's only being used by HomeKey and to add it externally of HomeKit again wouldn't make sense.
It was a bummer at the start but after reading the code and the HAP spec i did notice that no characteristic makes use of the WR perm or TLV which immediately made sens why you would choose not to add it and you will notice that the WR permission is not mentioned in my code as i didn't knew where that could fit and didn't bother figuring it out since the whole implementation for write response is also not very fitting but it works which is what mattered to me at the time and it's the reason why i just chose to pass along the base64 value instead of decoding it and parsing the TLV from inside HomeSpan.
Really appreciate the work you did on the new TLV8 code, it makes it easier and safer to work with TLV data, still don't like that i have to define an array with names for each tag though i love the concept.
We can definitely integrate into setString with a check for WR permission and make use of the "r" field that's parsed here which denotes a response is expected so we know to give the value that was previously set with setString and i think this could just be a boolean on the SpanBuf object that can picked up by printfAttributes
That is right, in order for the whole to "work as expected", it needs to retrieve the public key of all controllers for two reasons:
An overloaded pairCallback would be great. I understand that changing the signature would introduce a breaking change which i just added to call the pairCallback from addController, i wish could've done something like that for the callback 😅, it's basically drilling through entire functions right now 😂. Regarding the battery limitation, yeah, i know i've mentioned the battery on my README but reality is that is not going to come too soon for the exact same reason and i also haven't found any HAP library that has BLE implemented except for Apple's HomeKitADK which i don't really wanna use, don't know if it even works with ESP-IDF which is probably why Espressif made their own version. I'll see to make modifications to the code to incorporate what i've suggested above and perhaps try to integrate the TLV8 format a bit more. |
|
I'll add the WR and "r" for the write-response functionality and post an update to the dev branch in the next or so. |
rednblkx
changed the title
|
Update: I've added native support for the write-response mechanism. Code is available in the dev branch. As you can see from the latest commit, only a handful of lines needed to change (though it required a lot of thought and some surgical precision to keep things as generic as possible). HomeSpan now looks for an However, I found a terrific use for the WR permission. If you add it to any Characteristic, it forces the Home App to always request write-responses for that Characteristic when calling for updates. This even includes standard HAP Characteristics, which made it possible for me to test the code. I created a simple Lightbulb Service with On() and Brightness() Characteristics, and then added WR to the Brightness() permissions. The Home App started to send For your NFC Characteristics, it may be that the Home App will send proper There is (almost) nothing special you need to do to change the value of Characteristic subject to a write-response request. You will see that Characteristic updated in the In the next update I will add some additional logic that prevents using I plan to also add some tweaks to the STRING type to better support TLV Characteristics, since there is a hardcoded 64-character limit on STRINGS (which would include base64 TLV data) that will likely be too limiting. Give the dev branch a try and let me know how it goes. |
|
Fantastic! I finally gave it a go and i think it doesn't like me anymore or something 😂. Did a clean flash, not really sure what is going on, the only change i did was to add the Services and Characteristics included in this PR. This is the log, it's the same thing over and over I did also tried to create a new project, ran SimpleLightBulb and then just added the HardwareFinish Characteristic in order to isolate HomeSpan and make sure i did not messed something up but still bootlooped. Any idea what might be the issue? |
|
Sorry, subsequent to my note above I started working on native support for TLV and inadvertently pushed a work-in-progress to GitHub. For testing just the WR stuff, please use commit 41f7550. I will be working on the rest of the TLV stuff over the next week or so. |
Though this increases space requirement for TLV8 (just a little), using std::list allows adding elements directly to end, which ensure the order is maintained when packing and unpacking. Will avoid potential confusion when TLV8 is used for Characteristics.
Also truncated string Characteristics printed via 'i' CLI Command to show only first 32 characters followed by "..." if there are more than 32.
Works, but is memory-inefficient since it decodes entire string before unpacking. Need to add new functionality to TLV8 so that unpacking can be done in chunks similar to how pack() works. Also need to create getNewTLV() OR make all get/getNew generic to reduce code size.
This is much more memory efficient. Instead of decoding entire STRING from base64 to a temporary buffer or potentially very large size and then unpacking into TLV object, we decode a maximum of 48 characters at a time and unpack the resulting 36 (max) bytes until entire STRING is consumed. getTLV() returns pack_size of final TLV, unless there is a problem with conversion, in which cae TLV is wiped and return value=0
avoids unnecessary duplication of code where the only difference is whether you need the value or the newValue of a Characteristic.
Though this simplifies the code, the code size is still the same - compiler must have already optimized these functions.
Apple "escapes" forward slashes in JSON output, replacing '/' with '\/', which destroys base64 strings.
Allows easy-add of a sub TLV to an existing TLV8
Also fixed bug using increment operator (++) by replacing with std::next()
…) template for returning an integer
and a watchdog trigger prevention hack
|
Hello! It's me again, sorry for the delay, i see you have created a new branch tlvwork on top of latest dev branch commit so thought i would give that one a go since it includes the tlv format for characteristics. So far it seems to be working as intended, also checked the ios logs and , had an issue with setString but only because i was mistakenly passing something undefined 😅, thank you very much for your work. i rebased the branch to the tlvwork with my modification for the homekey characteristics on top, hope that doesn't complicate things. Now remains to also implement something for the paircallback, tried to do something a couple weeks but it got me into circular dependency hell so i left it for another time, do you have anything in mind for this? Again, thank you for the work you've done. |
|
I still have a little more work to do on the TLV stuff before I merge into the dev branch, including a dedicated page that documents all the TLV functions. I'll add a function to accomplish pairCallback at that time as well. Once completed you should not have to make any modifications to the library in order to implement a HomeKey. Should be able to everything from within a sketch, including adding new Services and Characteristics using the new CUSTOM_CHAR_TLV() macro. |
|
Ok, great, feel free to close this one, should’ve opened an issue instead but no point to it now. |
|
TLV8 work is now complete and documented. See the new TLV8.md page for details. You should now be able to add HomeKey-specific Services and Characteristics by using the CUSTOM_SERV() and CUSTOM_CHAR_TLV8() macros, as well as create TLV8 objects to use with those Characteristics. I've also exposed the full list of Controller data through the use of iterators. For example, the following function can be added to your sketch to loop over all Controller List data and print out the values: void list_controllers(){
Serial.printf("\nControllers\n");
for(auto it=homeSpan.controllerListBegin(); it!=homeSpan.controllerListEnd(); ++it){
Serial.printf("Admin=%d ID=",it->isAdmin());
for(int i=0;i<36;i++) // note length of ID is always 36 bytes
Serial.printf("%02X",it->getID()[i]);
Serial.printf(" LTPK=");
for(int i=0;i<32;i++) // note length of LTPK is always 32 bytes
Serial.printf("%02X",it->getLTPK()[i]);
Serial.printf("\n");
}
}I also added a Code is available in the tlvwork branch. Will move to dev branch after I finish documenting the Controller List stuff. Let me know how it works. If I've understood everything correctly, you should NOT have to make any modifications to the HomeSpan library itself to add HomeKey functionality to your sketch. If everything works and is reasonably robust, I'll add the HomeKey Services and Characteristics to the library as well so they can be used by everyone. |
|
Hello! I have now integrated the new APIs and i'll say that is working without issues so far, i've been running it for some days whilst i was on working on same other features and it's great also i see you've merged into Haven't used the new TLV8 APIs much but i appreciate that i can now set TLV data on a characteristic and let homespan encode it to base64 and everything else. Don't really have much to say except thank you for the stellar work you've done. Though i have some feedback for the TLV8 Serializer, i like the structure and i now use it(partially) in my HomeKey library but there is one issue, during the Attestation flow a huge data package of 719 bytes gets received in BER-TLV format, obviously you can't encode that length in a single byte and so three bytes are used for length as indicated by the BER encoding. Now, i know it might not make sense to modify the unpacking logic for this since TLV8 is it's own thing, just thought i might share this. |
|
@HomeSpan Is there any schedule for the public release of the dev branch? 😄 |
|
Not yet - there is a memory leak I am trying to resolve. |
Hi there!
Love the project and i'm currently using HomeSpan to build this which implements Apple's HomeKey into a ESP32 powered HomeKit lock.
In achieving the above i've had to make some modifications to HomeSpan which include the following:
I have also added some delays as a quick fix for a "task watchdog triggered" issue that occurs when using ESP-IDF built from source and not the version builtin with Arduino-ESP32.
Tried setting the EV permission for the NFCAccessControlPoint char in order to set it's value in hoping it would work as a response and the value would get set but it wouldn't send it, if i return true from update() it would send just 204 No Content and if i return false it would send 207 Multi-Status(which we need for WR) but the data would lack the value property and the status would be -70402 instead of 0 as i see that is tied to the return value here
I've no idea if this fits in with the project but thought i should still try to open a PR, i know the current code for the callback is at best a dirty hack but i'm willing to help integrate it or modify the set value functions for characteristics to accommodate write response.
Looking forward to hear your feedback.