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

Refactor SHT40 and Add SHT3x #325

Merged
merged 7 commits into from
Sep 21, 2022
Merged

Refactor SHT40 and Add SHT3x #325

merged 7 commits into from
Sep 21, 2022

Conversation

tyeth
Copy link
Contributor

@tyeth tyeth commented Sep 14, 2022

This refactors the SHT40 / SHT41 to use the Sensirion library arduino-sht which brings support for SHT3x SHTCx SHTWx.
Also added driver entry for SHT3x.

@brentru
Copy link
Member

brentru commented Sep 14, 2022

@tyeth Do you have these sensors to test with, or should I?

@tyeth
Copy link
Contributor Author

tyeth commented Sep 14, 2022

I've tested the SHT30 with address 44 and 45 (have initialised as 44 and code replacing SHT40 as SHT3X_ALT - address x45), then Add SHT40 and after i2c scan (address 44) and selecting frequency, but before choosing update component you swap the i2c address pin from GND to VCC and get it to address 45.

I don't have any SHT40's on hand, nor the adafruit SHT31-D, so if you or anyone else could test that would be most appreciated

@brentru
Copy link
Member

brentru commented Sep 14, 2022

I don't have any SHT40's on hand, nor the adafruit SHT31-D, so if you or anyone else could test that would be most appreciated

I have an SHT40 on my desk, and just ordered an SHT31-D from Adafruit to test. I will get to this by next week.

@@ -315,6 +315,17 @@ bool WipperSnapper_Component_I2C::initI2CDevice(
_sht4x->configureDriver(msgDeviceInitReq);
drivers.push_back(_sht4x);
WS_DEBUG_PRINTLN("SHT4X Initialized Successfully!");
} else if (strcmp("SHT3X", msgDeviceInitReq->i2c_device_name) == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we use lcase names here? The definition will need to reflect that, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah definitely, should I migrate the SHT40 too, as that's what suggested to me to go uppercase?

Copy link
Contributor Author

@tyeth tyeth Sep 15, 2022

Choose a reason for hiding this comment

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

Regarding this, I wonder if doing a git mv in the components repo will cause problems, I had to move to some intermediary name to get it to work in windows (expected unless you make the parent folder case aware) so I've committed it like that and done the 40 at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah definitely, should I migrate the SHT40 too, as that's what suggested to me to go uppercase?

Sure, and I'm not sure. I think it will just refresh the components once pulled in.

@lorennorman Will renaming an already-available component to from ucase to lcase cause issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, such a rename will cause issues. please let me know when you're ready to import that component name change, and i will take the necessary actions in the database.

we should tighten this up in the validators as well. all board and component names and directories need to be always lowercase (in my opinion.) easy to check and flag.

Copy link
Contributor Author

@tyeth tyeth Sep 16, 2022

Choose a reason for hiding this comment

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

ready when you are, or wait for @brentru next week to get his hardware, but I've soldered up my SHT30's 31's and 35's, so as long as 'we' (Brent) retest the sht40 which he already has in his possession then it'll have full coverage.

Copy link
Member

Choose a reason for hiding this comment

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

@tyeth This passes my test with SHT40. I'll merge this when the changes are made on the component repo's end, and OK with @lorennorman.

@brentru
Copy link
Member

brentru commented Sep 15, 2022

@tyeth Let me know via pinging me here when this is ready for re-review.

@tyeth
Copy link
Contributor Author

tyeth commented Sep 15, 2022

@brentru I think it's ready. I accidentally marked all conversations resolved, but as I mentioned I'm a bit of a newbie in cpp so please do mention the obvious / assumed...
I've done the rename on the component repo too, so probably best wait for confirmation of the upper/lowercase renaming being safe. Maybe also worth waiting for your sensor test next week, no rush here as I can doctor the firmware for my needs and drop the v50 bump commit.

@brentru brentru self-requested a review September 15, 2022 20:20
@brentru
Copy link
Member

brentru commented Sep 15, 2022

@tyeth Yeah, I added this to the list for next week's tasks. Should be ready to test on hardware by then.

@tyeth
Copy link
Contributor Author

tyeth commented Sep 15, 2022

Made a whoopsie and removed the SCD4x driver thinking it was SHT4x, was breaking builds on CI, restored.

@brentru
Copy link
Member

brentru commented Sep 19, 2022

Build failure due to release 2.0.5 today, not the PR's fault.

@tyeth
Copy link
Contributor Author

tyeth commented Sep 19, 2022

Cool, thanks for latest info regarding spurious failure + sht40 testing. The component change is also there under adafruit/Wippersnapper_Components#77

@tyeth
Copy link
Contributor Author

tyeth commented Sep 20, 2022

@brentru The component change has gone through and seems to be working fine for me, added a comment in that pull request.
Probably worth testing if v49 works with the new lowercase name and if not then getting v50 out with the new lowercase name check as soon as possible. Thanks for your help on this.

@brentru
Copy link
Member

brentru commented Sep 21, 2022

Merged #329, rebuilding on affected platform.

@brentru brentru merged commit bf59144 into adafruit:main Sep 21, 2022
@tyeth
Copy link
Contributor Author

tyeth commented Sep 22, 2022

@brentru I've just been struggling to put the firmware (v50) on the nearest QTPY S2. It compiles and uploads fine with a cloned repo and the Arduino IDE, but using the official v50 release UF2 file for qtpy ESP32-s2 copies and then gets stuck with solid purple light forever on each boot. Like the uf2 file is broken... I tried the file from github and via Wippersnapper update page

@TheGodMotherG
Copy link

Any news on the update I am using in my drying room and am fully in the dark now. I feel like such a fool for updating it when it was working great. Any eta on time. Or an idea on how to maybe use arduino to get an old version running I really need it. I know it's silly to put my livelyhood on one arduino controller but I did. So I'm freaking out all day

@tyeth
Copy link
Contributor Author

tyeth commented Sep 23, 2022

Any news on the update I am using in my drying room and am fully in the dark now. I feel like such a fool for updating it when it was working great. Any eta on time. Or an idea on how to maybe use arduino to get an old version running I really need it. I know it's silly to put my livelyhood on one arduino controller but I did. So I'm freaking out all day

In your original ticket 4hours ago 😀 Should come up in the normal wippersnapper update pages, or under the releases section of the github repo

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 23, 2022 via email

@tyeth
Copy link
Contributor Author

tyeth commented Sep 23, 2022

Oooh no idea on that one. I have noticed my secrets.json gets messed up sometimes usually when going between versions but sometimes disconnecting while writing, so I'm guessing it might be worth me erasing the flash once I get in a corrupted state. I dont, instead I just save a fresh copy of secrets.json and reboot the board, normally fixes everything, if not then reapply firmware again. If it still fails then apply 49 and should be okay. If that fails then go back to 44 also somewhere on the releases page 🤞

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 23, 2022 via email

@tyeth
Copy link
Contributor Author

tyeth commented Sep 23, 2022

This is worth sticking in a new bug ticket. Looks like your file system is corrupted after the update, like my secrets.json file is now after using latest release. I guess the storage memory location moved a bit and without using Erase Flash and setting up secrets again we just cross our fingers each time and hope for no corruption.

You could try just writing the Credentials file again and that may sort it
image

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 23, 2022 via email

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 23, 2022 via email

@tyeth
Copy link
Contributor Author

tyeth commented Sep 23, 2022

Ah, that is my fault, sort of. I added the SHT35/31/30, and while doing so was told to lowercase it, and the SHT40 too which was previously uppercase. The older firmware versions wont know about lowercase naming of the sht40 sensor. You will have to do a new bug/issue mentioning the v50 issue. They'll(@brentru) want to sort it quickly for the feather ESP32 v2 anyway...
It's probably worth getting the arduino IDE working if you're happy getting your hands dirty as it's a one line change (just 307) main...tyeth:Adafruit_Wippersnapper_Arduino:cheeky-sht40-30-swap#diff-3dd55e689e75bcb8859a65746742c85d8b634ee427ba635ece3bd5d91711eee6R307

@TheGodMotherG
Copy link

i tried to do the Arduino but keep getting a compiling error after i put in my credentials

@brentru
Copy link
Member

brentru commented Sep 23, 2022

Since this is a new bug (not related to what was solved yesterday), we're looking into this today with no ETA. Something between the Web Firmware Uploader and the new Arduino board support package is causing an issue. We haven't had to handle tests around the uploader specifically and haven't seen this type of bug before. This bug gives us an opportunity to add tests and increase stability so it doesn't happen again.

Adafruit IO WipperSnapper is in beta and is actively being developed to add functionality, new hardware, and fix bugs. We do not want to break your project, and we are sorry given the situation you are in. We encourage you to try out WipperSnapper firmware with the understanding that _it is not the final release software yet (beta).

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 23, 2022 via email

@tyeth
Copy link
Contributor Author

tyeth commented Sep 24, 2022

You could very quickly chuck circuitpython (micropython alternatively) on it, add wifi secrets, include the scd40 circuit python library with example code and publish to the same feed (mqtt adafruit io example). You'll be pleasantly surprised how easy it comes together. I only moved to wippersnapper due to the number of devices and sensors (setting up feeds is painful if you add descriptions and sensible names). Check out the circuitpython web workflow, basically you get a file manager and serial terminal in the browser (via ip or http://circuitpython.local)

This is overkill (wanted to try calling sensor callbacks) and using different sensors, but I wanted to publish two SHT35's and two ADS1015 4channel ADC's to adafruit io, and they all error occasionally. I wanted to re-read the old values too initially, and there is code in there to jump around networks if the main one isn't there (I have 3networks to cover front of house to back greenhouse). You will have an easier time just using the scd40 example and adafruit io example, but this may help/interest you:
https://github.com/tyeth/circuitpython-micropython-quicktest-qtpy-esp32-s2/blob/main/qtpys2_io_adc_sht3x/

The feedname I used to retrieve the scd40 co2 that wippersnapper is publishing, is
tyeth/feeds/qtpy-esp32s219440171.ws-0x62-co2
image
BTW: QoS = 2 is unsupported on adafruit at the moment, 0 or 1 only in theory.

@TheGodMotherG
Copy link

TheGodMotherG commented Sep 24, 2022 via email

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.

4 participants