Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Preparation for library based QR-Code generation - setup identifier #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Preparation for library based QR-Code generation - setup identifier #2

wants to merge 3 commits into from

Conversation

cduvenhorst
Copy link

  • Added hap.setupid configuration option.
  • Moved display and nfc event emitting functions for transparency to the corresponding platform abstraction layer files
  • Added setup identifier loading for hap simple configuration. If none was supplied, generate a unique one.

@cduvenhorst
* Added hap.setupid configuration option.
37fef2b 
* Moved for transparency display and nfc event emitting functions to the corresponding platform abtraction layer files
* Added setup identifier loading for hap simple configuration
cduvenhorst
cduvenhorst commented Oct 25, 2020
View reviewed changes
src/mgos_homekit_adk.c Outdated

char* err = NULL;
save_cfg(&mgos_sys_config, &err);
printf("Saving configuration: %s\n", err ? err : "no error");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops. Will be corrected ..

@rojer
Copy link
Contributor

rojer commented Oct 25, 2020

looks good!
make sure RPC handler is updated to handle this. i.e., HAP.Setup should be modified to set code, HAP.Reset should clear it.

@rojer
Copy link
Contributor

rojer commented Oct 25, 2020

actually, can you describe the flow of the setup with QR code? starting with a blank device, what will happen?

@cduvenhorst
Copy link
Author

Ok! Meanwhile I ran into a problem which seems to be caused by the dynamic buffer allocation or the helper functions in the HAP lib. Have to dig a little deeper to nail it. Pairing fails based on dynamic setup info.

@rojer
Copy link
Contributor

rojer commented Oct 27, 2020

sounds good. when chasing a memory leak or corruption issue, using "grown up" tools such as address sanitizer is invaluable.
mongoose os has a minimal "ubuntu" platform, that makes it possible - it doesn't support peripherals, but is ideal if you can separate the code you're debugging into something that doesn't require hardware access.
and for shelly-homekit, we have a "ShellyU" target that is exactly that - it's a "switch" with no physical connections required but web interface and HAP functionality is fully working. it will catch memory corruption and pointer misus, such as double-free or use-after-free. really invaluable.
long story short, here's how to build and use it:

on a Ubuntu machine with clang installed (apt install clang), run

make ShellyU

in the main source directory, then run the binary:

ASAN_SYMBOLIZER_PATH=$(realpath /usr/bin/llvm-symbolizer-*) build_ShellyU/objs/shelly-homekit.elf --insecure --chroot build_ShellyU/fs

go to http://localhost:8080/ to access the web interface, pair, etc. if your firmware does something nasty with a pointer, it will crash and you'll get a full trace.
leak check is only performed on graceful exit, so to check if it leaks memory press ctrl-c in the terminal and if it does, you'll get a report.

@cduvenhorst
* Major rewrite of rpc setup handler to reflect payload generation an…
dd48a96 
…d honor the setting of a fixed setup identifier

* Added mgos_hap_setup_id_from_string(..) to mgos_hap.h
* Fixed persistence of setup identifier for a given config level
* Removed a stale „printf“
@cduvenhorst
* Fixed Log level
316d3f0
@cduvenhorst
Copy link
Author

I will update example-homekit for a POC.

cduvenhorst
cduvenhorst commented Nov 1, 2020
View reviewed changes
src/mgos_homekit_adk_rpc_service.c
.bleSupported = (server->transports.ble != NULL) };
HAPAccessoryCategory category = s_acc->category;

LOG(LL_INFO,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaking sensitive info ... will be fixed.

@cduvenhorst
Copy link
Author

Ok! Meanwhile I ran into a problem which seems to be caused by the dynamic buffer allocation or the helper functions in the HAP lib. Have to dig a little deeper to nail it. Pairing fails based on dynamic setup info.

Ok. This is caused by memory limitations if building for esp8266. The esp32 platform just runs fine with dynamic setup codes.

@rojer
Copy link
Contributor

rojer commented Nov 1, 2020

i can take a look, just tell me how to test

@cduvenhorst
Copy link
Author

i can take a look, just tell me how to test

Strange. If you activate the accessory setup info server (set setupDisplay in happlatform) everything works with esp32, but esp8266 will not pair. I suspect memory limitations.

I have updated my example-homekit fork on branch "qrcode" for you to test.

Simply provision wifi and head over to "http://<ip>/homekit/pairing" and try to pair while having a look at the console.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cduvenhorst @rojer