Skip to content

Conversation

@Jc2k
Copy link
Collaborator

@Jc2k Jc2k commented Aug 25, 2019

As promised here is the other half of #161. The perform_pair_setup_part* functions are now 'pure' state machines - they don't do the IO themselves, and this means we can reuse them with the async client in #154. When this lands i will be able to completely rm homekit/aio/controller/ip/crypto.py from that branch.

(And this does eliminate create_ble_pair_verify_write again).

sys.exit(-1)

write_fun = create_ble_pair_verify_write(pair_verify_char, pair_verify_char_info['iid'])
write_fun = create_ble_pair_setup_write(pair_verify_char, pair_verify_char_info['iid'])
Copy link
Owner

Choose a reason for hiding this comment

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

so we do not have separate functions for create_ble_pair_verify_write and create_ble_pair_setup_write anymore, can we find a better name? perhaps create_characteristic_write_function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can actually factor out the need for the create_ part entirely really, but i didn't want to over complicate this PR.

Now that we aren't passing it into the state machine we don't need to capture the scope. So it could just be a generic write helper like (pseudo-code, won't work as is):

def write_hap_param_value(char, iid, request, expected):
    pass

...

def setup_session():
    char, iid = find_char_by_type(PAIR_VERIFY)
    state_machine = ...
    while True:
        response = write_hap_param_value(char, iid, request, expected)
        request, expected = state_machine.send(response)

@jlusiardi
Copy link
Owner

I will try to perform some tests with a real hardware!

@jlusiardi
Copy link
Owner

Done and lgtm

@jlusiardi jlusiardi merged commit ebe16c9 into jlusiardi:master Sep 6, 2019
@Jc2k
Copy link
Collaborator Author

Jc2k commented Sep 6, 2019

Wahhooooo

@Jc2k Jc2k deleted the pair_setup_state_machine_refactor branch September 6, 2019 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants