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

Basic implementation of ropot #111

Merged
merged 8 commits into from
Sep 1, 2019
Merged

Basic implementation of ropot #111

merged 8 commits into from
Sep 1, 2019

Conversation

chrostek
Copy link
Contributor

@chrostek chrostek commented Jul 2, 2018

very basic implementation of ropot. gives back false for nonexistant value MI_LIGHT for ropot

very basic implementation of ropot. gives back false for nonexistant value MI_LIGHT for ropot
@chrostek chrostek mentioned this pull request Jul 2, 2018
@ChristianKuehnel
Copy link
Collaborator

@chrostek Thx for your contribution 👍 ! I just had a look and the change really is quite minimal. Nice!

I do have two requests for your pull request before merging it:

  1. You're using the length of the data to identify if we're talking to a normal sensor or to a ropot. Please move this check into a new function "is_ropot()" or something similar so that it's clear why you're doing this check and then use this function instead of the len(data)>24 check, where you need to check.

  2. Please add a test case with some test data for a ropot so that we make sure this use case keeps working in case of future changes.

@ChristianKuehnel
Copy link
Collaborator

close/open to trigger a new build

@chrostek
Copy link
Contributor Author

I'm no python programmer, so i asked a friend to implement your request. unfortunately my ropot seems to be dead (no bt connection to app or miflora anymore), so i cannot test the new changes.

@ChristianKuehnel ChristianKuehnel merged commit d9c3831 into basnijholt:master Sep 1, 2019
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.

None yet

3 participants