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

Add support for HTU21D/SHT21 temperature and humidity sensors (ESP32) #2250

Closed
wants to merge 2 commits into from

Conversation

pawel-sw
Copy link

@pawel-sw pawel-sw commented Feb 4, 2018

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

HTU21D is cheap digital temperature and humidity sensor.
Module for ESP32

@pawel-sw pawel-sw changed the title Add support for HTU21D/SHT21 temperature and pressure sensors Add support for HTU21D/SHT21 temperature and pressure sensors (ESP32) Feb 4, 2018
@pawel-sw pawel-sw changed the title Add support for HTU21D/SHT21 temperature and pressure sensors (ESP32) Add support for HTU21D/SHT21 temperature and humidity sensors (ESP32) Feb 4, 2018
@devsaurus
Copy link
Member

Thanks for your contribution @s-pw!

Comparing this PR with #2249 I see that you introduced a platform_htu21_* layer for ESP32. The same pattern is visible in your PRs for BH1750. IMO this is a valid approach since I2C interfaces are not yet fully abstracted to platform layer on ESP32. Which means:

  • platform_htu21 (and the Lua) module is restricted to i2c.SW interface
  • support for i2c.HW0|1 can be added in the future to components/platform/htu21.c
  • once platform_i2c_* also abstracts the HW interfaces, then platform_htu21 can be simplified or even obsoleted

But there is currently no way to distinguish the I2C interface. Therefore I propose that you add the i2c id to the APIs on Lua and platform level:

  • htu21.read(i2c_id)
    The Lua module passes id to platform_htu21 and checks for PLATFORM_OK result
  • platform_htu21_read(unsigned id, uint8_t reg)
    Checks for support of requested id and returns PLATFORM_ERR if not supported.

@pawel-sw
Copy link
Author

pawel-sw commented Mar 4, 2018

Thanks, I like the idea. I'll update modules for both ESP32 and ESP8266. ESP8266 won't be just for compatibility as the sensor supports speed up to i2c.FAST (400kHz). There is software i2c interface that could support i2c.FAST and i2c.FASTPLUS : https://github.com/pasko-zh/brzo_i2c

@stale
Copy link

stale bot commented Jan 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 8, 2020
@marcelstoer
Copy link
Member

This applies to #2249, #2250 (this one), #2259, and #2261

@devsaurus @pjsg Do you have any idea why these modules were never merged? Was it because of @TerryE's comment #2261 (comment)?

@stale stale bot removed the stale label Jan 9, 2020
@jpeletier jpeletier mentioned this pull request May 11, 2020
4 tasks
@stale
Copy link

stale bot commented Jun 16, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 16, 2021
@stale stale bot closed this Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants