Update AM2320 documentation with frequency note #5522
Conversation
Added note about sensor frequency for improved reliability, and allow it to work.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the AM2320 sensor documentation to address reliability issues caused by I2C bus frequency settings. The primary purpose is to help users avoid common configuration problems by documenting that the AM2320 sensor requires a lower I2C bus frequency than ESPHome's default.
Key changes:
- Added frequency recommendation note explaining the 100kHz limitation and default 200kHz issue
- Expanded example configuration to show complete I2C bus setup with recommended 50kHz frequency
- Added explicit i2c_id and address configuration in the sensor example
| > [!NOTE] | ||
| > Logs might include some warnings about receiving a NACK from the sensor. | ||
| > This is due to a wake call to the sensor which the sensor never acknowledges by design. | ||
| > Sensor works on BUS frequency up to 100kHz, but by default is 200kHz in use. So decrease will help with sensor reliability. |
There was a problem hiding this comment.
The note should explain why 50kHz is recommended when the sensor supports up to 100kHz. Consider revising to: 'The sensor supports I2C bus frequencies up to 100kHz. ESPHome's default frequency is 200kHz, which may cause reliability issues. Setting the frequency to 50kHz (well below the maximum) ensures stable operation.'
| > Sensor works on BUS frequency up to 100kHz, but by default is 200kHz in use. So decrease will help with sensor reliability. | |
| > The sensor supports I2C bus frequencies up to 100kHz. ESPHome's default frequency is 200kHz, which may cause reliability issues. Setting the frequency to 50kHz (well below the maximum) ensures stable operation. |
There was a problem hiding this comment.
As far as I know the default is 50kHz? Where do you see a default of 200?
There was a problem hiding this comment.
I just see there in the logs from some esp device. Maybe this is the default in some sensor library. Lets change the notes as needed.
| sda: GPIO4 | ||
| scl: GPIO5 | ||
| scan: true | ||
| frequency: 50000 |
There was a problem hiding this comment.
[nitpick] Consider adding a comment in the YAML example explaining the frequency value, such as frequency: 50000 # 50kHz - recommended for AM2320 reliability
| frequency: 50000 | |
| frequency: 50000 # 50kHz - recommended for AM2320 reliability |
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughDocumentation update for the AM2320 sensor component adding a clarification note about I²C frequency operation limits and expanding the configuration example with I²C bus ID, frequency, and device address settings. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
content/components/sensor/am2320.md (1)
17-20: Improve grammar and clarity of the frequency note.The note provides important troubleshooting information, but the phrasing on line 20 could be clearer. The current text "So decrease will help with sensor reliability" is grammatically awkward and could be misunderstood.
Apply this diff to improve clarity:
> [!NOTE] > Logs might include some warnings about receiving a NACK from the sensor. > This is due to a wake call to the sensor which the sensor never acknowledges by design. - > Sensor works on BUS frequency up to 100kHz, but by default is 200kHz in use. So decrease will help with sensor reliability. + > The AM2320 sensor operates reliably at I²C frequencies up to 100 kHz. ESPHome defaults to 200 kHz, which may cause intermittent communication issues. Reducing the I²C frequency improves reliability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
content/components/sensor/am2320.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
- Do not generate or add any sequence diagrams
Files:
content/components/sensor/am2320.md
🔇 Additional comments (1)
content/components/sensor/am2320.md (1)
24-39: Configuration example is good, but document the new options.The enhanced configuration example is clear and correct:
- The i2c bus definition with
frequency: 50000(50 kHz) is well below the 100 kHz sensor limit, which is appropriately conservative.- Explicit
i2c_idandaddressreferences are helpful for clarity.However, the Configuration variables section (lines 42–52) does not document the newly added
i2c_idandaddressoptions. If these are not inherited from parent component documentation, they should be added to the configuration variables list.Please verify: Are
i2c_idandaddressdocumented in the general I²C component documentation and Sensor base documentation respectively? If not, add them to the Configuration variables section here.
| > [!NOTE] | ||
| > Logs might include some warnings about receiving a NACK from the sensor. | ||
| > This is due to a wake call to the sensor which the sensor never acknowledges by design. | ||
| > Sensor works on BUS frequency up to 100kHz, but by default is 200kHz in use. So decrease will help with sensor reliability. |
There was a problem hiding this comment.
As far as I know the default is 50kHz? Where do you see a default of 200?
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
2 participants
Added note about sensor frequency for improved reliability, and allow it to work.
Description:
I spend a lot of time with searching for why the sensor AM2320 doesn't work with ESPhome, but with arduino code yes. I tried many solutions, like add there setup prio -100, or pull up the SCL right after the boot, but nothing of that work correctly and reliably. After that I found a documentation for AM2320 where is that maximal BUS frequency is 100kHZ, and thats it. Just decrease the frequency from default 200kHz to 50kHz make the magic.
So would be perfect add this info to official FAQ to help others.
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable):
Checklist:
I am merging into
nextbecause this is new documentation that has a matching pull-request in esphome as linked above.or
I am merging into
currentbecause this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.Link added in
/components/index.rstwhen creating new documents for new components or cookbook.New Component Images
If you are adding a new component to ESPHome, you can automatically generate a standardized black and white component name image for the documentation.
To generate a component image:
Comment on this pull request with the following command, replacing
COMPONENT_NAMEwith your component name in UPPER_CASE format with underscores (e.g.,BME280,SHT3X,DALLAS_TEMP):The ESPHome bot will respond with a downloadable ZIP file containing the SVG image.
Extract the SVG file and place it in the
images/folder of this repository.Use the image in your component's index table entry in
/components/index.rst.Example: For a component called "DHT22 Temperature Sensor", use: