Minor issues #2
Open
Minor issues #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
castilma
commented
May 18, 2017
| @@ -38,25 +38,23 @@ def get_DHT11_data(): | |||
| # guarantee the timing of calls to read the sensor). | |||
| # If this happens try again! | |||
|
|
|||
| for i in range(1, 3): | |||
| tries = 2 | |||
| for i in range(1, tries + 1): | |||
There was a problem hiding this comment.
The comment in line 31 says read_retry() is being used, but read() is used. And we're only trying 2 times.
| print('started') | ||
| try: | ||
| humidity, temperatureDHT = Adafruit_DHT.read( | ||
| Adafruit_DHT.DHT11, 17) | ||
| if humidity is not None and temperatureDHT is not None: | ||
| return humidity, temperatureDHT | ||
| sleep(2) | ||
| print('{}. DHT Measurement failed. Starting next try'.format(i)) |
There was a problem hiding this comment.
This prints [...] Starting next try even after the last try.
| except ValueError: | ||
| print('Sensor not found') | ||
| break |
There was a problem hiding this comment.
When there is no sensor, there is no reason to retry, right?
| # Note that sometimes you won't get a reading and | ||
| # the results will be null (because Linux can't | ||
| # guarantee the timing of calls to read the sensor). | ||
| # If this happens try again! |
There was a problem hiding this comment.
This is a copy of lines 36-39.
| print('Error creating folder: ' + str(e) ) # errno 17= File existed | ||
| else: | ||
| pass | ||
| print('Folder created') |
There was a problem hiding this comment.
Only print the message if makedirs() succeded. Otherwise we didn't create the folder.
| @@ -137,16 +135,16 @@ def logging(): | |||
| print(folderName) | |||
| try: | |||
| os.makedirs(folderName, mode=0o777) | |||
There was a problem hiding this comment.
Really mode 0o777? How about at most 0o775?
| if e.errno != 17: | ||
| print('Error creating folder: ' + str(e) ) # errno 17= File existed | ||
| else: | ||
| pass |
There was a problem hiding this comment.
Maybe this is a good situation to abort? But maybe not. I don't know.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found some minor things. See the review for details.