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

AHT10: Use state machine to avoid blocking delay #6401

Merged
merged 3 commits into from
Mar 19, 2024

Conversation

clydebarrow
Copy link
Contributor

What does this implement/fix?

The aht10 sensor uses delays in update() to wait for the chip to become ready to read. This causes unacceptable blocking of the loop() thread.

This PR replaces the in-line delays with scheduled callbacks to eliminate the thread blocking.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

# Example config.yaml
sensor:
  - platform: aht10
    variant: aht20
    update_interval: 15s
    humidity:
      name: "Humidity"
    temperature:
      name: "Temperature"

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@jesserockz jesserockz added this to the 2024.3.0b5 milestone Mar 19, 2024
@jesserockz jesserockz merged commit f0936dd into esphome:dev Mar 19, 2024
55 checks passed
@jesserockz jesserockz mentioned this pull request Mar 20, 2024
@jesserockz jesserockz mentioned this pull request Mar 20, 2024
@heintjeput
Copy link

I just updated my ESP32C3 with a AHT21 sensor to ESPhome 2024.3.0, it compiles fine, but the sensor doesnt seem to update anymore. Could it be related to this PR?

@clydebarrow
Copy link
Contributor Author

clydebarrow commented Mar 20, 2024

It's possible. Do you have any logs? And yaml?

@clydebarrow
Copy link
Contributor Author

There was another PR on this component, #6303 and I think there is an error in it. It's writing the wrong number of bytes in the init command. It works ok here on an aht20, but it's possible the 21 is affected.

@clydebarrow
Copy link
Contributor Author

@heintjeput Add this to your yaml and retest - there are a couple of changes there.

external_components:
  - source: github://clydebarrow/esphome@aht10-1
    refresh: 10min
    components: [ aht10]

@heintjeput
Copy link

heintjeput commented Mar 20, 2024

Thanks, for your replies. I think you are correct that it was in the other PR, I stopped searching at the first PR in the release.
I first reverted to the old PR#5198 there it works. With the current release I get the following in my log:
[11:17:40][C][aht10:164]: AHT10:
[11:17:40][C][aht10:165]: Address: 0x38
[11:17:40][E][aht10:167]: Communication with AHT10 failed!

The yaml is here.
Basically:

i2c:
  - sda: 18
    scl: 19
    #scan: True
    #id: bus_a
    #frequency: 10kHz

  - platform: aht10
    variant: aht20
    temperature:
      name: "AHT21Temperature"
    humidity:
      name: "AHT21Humidity"
    update_interval: 5s
    id: aht21
    address: 0x38

I am trying your improvement now, it might work, but let me check properly. I do get a value now

[11:26:08][V][sensor:043]: 'AHT21Temperature': Received new state 21.785355
[11:26:08][D][sensor:094]: 'AHT21Temperature': Sending state 21.78535 °C with 2 decimals of accuracy
[11:26:08][V][json:038]: Attempting to allocate 512 bytes for JSON serialization
[11:26:08][V][json:058]: Size after shrink 84 bytes
[11:26:08][V][mqtt:478]: Publish(topic='omvormer/sensor/aht21temperature/state' payload='21.79' retain=1 qos=0)
[11:26:08][V][sensor:043]: 'AHT21Humidity': Received new state 77.243614
[11:26:08][D][sensor:094]: 'AHT21Humidity': Sending state 77.24361 % with 2 decimals of accuracy
[11:26:08][V][json:038]: Attempting to allocate 512 bytes for JSON serialization
[11:26:08][V][json:058]: Size after shrink 80 bytes
[11:26:08][V][mqtt:478]: Publish(topic='omvormer/sensor/aht21humidity/state' payload='77.24' retain=1 qos=0)
[11:26:08][W][component:232]: Component aht10.sensor took a long time for an operation (62 ms).
[11:26:08][W][component:233]: Components should block for at most 30 ms.

However it is about 1 - 2 degrees higher than expected.

@heintjeput
Copy link

The issue is solved for me. Thanks for the help.

@cptskippy
Copy link
Contributor

There was another PR on this component, #6303 and I think there is an error in it. It's writing the wrong number of bytes in the init command. It works ok here on an aht20, but it's possible the 21 is affected.

I wrote PR #6303, the only functional change I made was to include the Soft Reset command because my AHT10s were giving the communication failure error message after firmware updates. Per the datasheets, the Soft Reset command is only 1 byte, where as the Trigger Measurement command is 3 bytes.

PR #6401 seems to have introduced a regression because my sensors began reporting the comms failure after updating their firmware:

[12:50:15][C][aht10:164]: AHT10:
[12:50:15][C][aht10:165]:   Address: 0x38
[12:50:15][E][aht10:167]: Communication with AHT10 failed!

@clydebarrow I added your branch to my yaml file and that seems to have resolved the issue. After firmware update my AHT10s are no longer reporting the comms failure and are sending out data again.

@clydebarrow
Copy link
Contributor Author

where as the Trigger Measurement command is 3 bytes.

Yes, but your PR writes 4 bytes as it takes size of a pointer instead of the data array. That tested ok on my AHT20, but might have different effects on other devices, and the value of the extra byte written is undefined so might vary from build to build as well. #6409 based on the branch you tested fixes that.

@cptskippy
Copy link
Contributor

Yes, but your PR writes 4 bytes as it takes size of a pointer instead of the data array.

🤦‍♂️ Doh! You're right. That probably did break it.

When I was writing #6303 I tried reducing the size of the Initialization command because the datasheets don't specify that it's 3 bytes, and when I did it failed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants