Issues addressed, working temperature sensor code #127
Conversation
…code for adafruit bme680 temperature sensor
There was a problem hiding this comment.
There are several folders (the dist ones) and files (.mpy) that aren't necessary or are unused. Please remove. Please also remove any dependencies that aren't used (e.g., I see you have robust and simple, but the implementation should be with mqtt_as). If you're not using mqtt_as yet, you can refer to https://ac-microcourses.readthedocs.io/en/latest/courses/hello-world/1.4-hardware-software-communication.html and other training lab IoLT examples.
Please move this from scripts to https://github.com/AccelerationConsortium/ac-training-lab/tree/main/`src/ac_training_lab/picow/temp-humidity-pressure-gas-sensor` (you'll need to create the directory).
You committed secrets. Please rename to my_secrets.py locally and then add my_secrets.py to .gitignore so that you're not committing the secrets. These secrets should then be imported, not defined directly.
You might not need urequests_2.py anymore. I think you can just use regular requests as normal. Please double check though.
Please update and then ping me when ready for me to review again.
|
Also, I'm not quite sure what the differences between this and #63 are. If these are duplicates, please close one and use a single branch / PR. |
…tting for better readability
…; include pre-commit configuration and license file
|
I'm not quite sure what's causing the merge conflict, as I took care of all the warnings that the pre-commit gave (flake, black, etc.). However, when I run flake, it gives me errors from other files (on occasion). When I run pre-commit, everything passes safely. |
There was a problem hiding this comment.
Thanks for the updates!
There shouldn't be a separate pre-commit file for tempSensor: scripts/tempSensor/.pre-commit-config.yaml. I deleted
pico_id typically isn't something you should expose: scripts/tempSensor/pico_id.txt. I deleted
Again, you shouldn't have robust.py and simple.py in there, since you should only be using mqtt_as.py. Please adjust so that you're using mqtt_as.py, and you can follow the general approach shown in https://ac-microcourses.readthedocs.io/en/latest/courses/hello-world/1.4-hardware-software-communication.html and the module 4 assignment that you should have completed already.
If the LICENSE is attached with the bme680.py script, then please change the name of the LICENSE file to LICENSE_bme680.txt or put the LICENSE and bme680.py file within its own subdirectory.
Once you take care of this, and run pre-commit again, if it still isn't passing I'll go in and fix. Also, after making the changes, please verify that it still works with the corresponding HF Space.
|
Aside: Next time it would be best to do this as a branch in ac-training-lab rather than in your own fork because I don't have permission to push to your fork and I had to initialize a new codespace. |
|
Ok, sorry about the conflicts. I will fix them as soon as possible. |
scripts/tempSensor/main.py
Outdated
| from bme680 import BME680_I2C | ||
| from machine import I2C, Pin, reset | ||
| from netman import connectWiFi | ||
| from umqtt.simple import MQTTClient |
There was a problem hiding this comment.
The script should be using mqtt_as, not mqtt.simple
scripts/tempSensor/main.py
Outdated
| from umqtt.simple import MQTTClient | ||
|
|
||
| # Configuration | ||
| SSID = "Pixel 8" |
There was a problem hiding this comment.
Please put all of this in a .gitgnore'd my_secrets.py file. You're exposing credentials publicly by committing.
scripts/tempSensor/main.py
Outdated
| # Configuration | ||
| SSID = "Pixel 8" | ||
| PASSWORD = "123456789" | ||
| MQTT_BROKER = b"b6bdb89571144b3d8e5ca4bbe666ddb5.s1.eu.hivemq.cloud" |
There was a problem hiding this comment.
Shouldn't need the b, at least in mqtt_as
scripts/tempSensor/main.py
Outdated
| bme = BME680_I2C(i2c) | ||
|
|
||
|
|
||
| def connect_to_wifi(): |
There was a problem hiding this comment.
Not sure why you have an additional wrapper to connect WiFi. Maybe overkill
…d async data publishing; add .gitignore for my_secrets.py
|
Nice, thanks! Have you verified this works in tandem with HF? If so, I think we're good to merge. |
|
Yes, this code works with the hf. Should I also include this code under a /scripts file for the space? |
|
That's OK, as long as the HF code is current. Can you copy the link to the HF space here? |
No description provided.