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.

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

Fix for Xiaomi Aqara gateway #101

Merged
merged 7 commits into from
Jan 31, 2019
Merged

Fix for Xiaomi Aqara gateway #101

merged 7 commits into from
Jan 31, 2019

Conversation

quarcko
Copy link
Contributor

@quarcko quarcko commented Jan 25, 2019

This gateway sends all his characteristics and services using lowercase letters.
Thus your library cannot identify some of information.
Quick fix to make uppercase-comparisons despite input being lowercase.

Some devices send their services and characteristics in lowercase:
Xiaomi aqara gateway (Homekit version)
Some devices send their services and characteristics in lowercase
Xiaomi Aqara gateway (Homekit version)
@jlusiardi
Copy link
Owner

Hi @quarcko

thank you for your PR, but we have to fix the errors in the tests first. Can you give me an example what the Xiaomi Aqara exactly returns? I guess just making stuff upper case in the place you propse can't work all the way down, because the input may also be an integer.

Regards
Joachim

@quarcko
Copy link
Contributor Author

quarcko commented Jan 25, 2019

Here is a diff between output with 'upper()' and without:
https://www.diffchecker.com/KH5pLojQ

I think i just overreacted with it. It is enough to make 'upper()' only in get_short()
of both characteristics and service types... no need to alter getitem, it still works as expected.

but:
FAIL: test_get_short (tests.characteristicsTypes_test.TestCharacteristicsTypes)

Traceback (most recent call last):
File "/home/travis/build/jlusiardi/homekit_python/tests/characteristicsTypes_test.py", line 51, in test_get_short
self.assertEqual(CharacteristicsTypes.get_short('1a'), 'Unknown Characteristic 1a')
AssertionError: 'lock-management.auto-secure-timeout' != 'Unknown Characteristic 1a'

  • lock-management.auto-secure-timeout
  • Unknown Characteristic 1a

This test is strange, you seem that on purpose do not allow 1a to be resolved like 1A? why?

@jlusiardi
Copy link
Owner

good point, not sure, can you extend the pull request like this:

        self.assertEqual(CharacteristicsTypes.get_short('1a'), 'lock-management.auto-secure-timeout')

Perhaps we should check if anything being a UUID is ok upper and lower case

@jlusiardi jlusiardi merged commit 9604d76 into jlusiardi:master Jan 31, 2019
@jlusiardi
Copy link
Owner

Thank you!

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