Skip to content

Conversation

swoboda1337
Copy link
Member

Description:

Add note about I2C errors and short update intervals

Related issue (if applicable): fixes esphome/esphome#10769

Pull request in esphome with YAML changes (if applicable):

  • esphome/esphome#

Checklist:

  • I am merging into next because this is new documentation that has a matching pull-request in esphome as linked above.
    or

  • I am merging into current because 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.rst when 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:

  1. Comment on this pull request with the following command, replacing COMPONENT_NAME with your component name in UPPER_CASE format with underscores (e.g., BME280, SHT3X, DALLAS_TEMP):

    @esphomebot generate image COMPONENT_NAME
    
  2. The ESPHome bot will respond with a downloadable ZIP file containing the SVG image.

  3. Extract the SVG file and place it in the images/ folder of this repository.

  4. Use the image in your component's index table entry in /components/index.rst.

Example: For a component called "DHT22 Temperature Sensor", use:

@esphomebot generate image DHT22

@esphome esphome bot added the current label Sep 22, 2025
Copy link
Contributor

coderabbitai bot commented Sep 22, 2025

Walkthrough

Documentation update to the QMC5883L sensor page: the update_interval field description now warns that very short intervals may cause I2C errors. No code, configuration, defaults, or behavior changes.

Changes

Cohort / File(s) Summary
Documentation: QMC5883L sensor
content/components/sensor/qmc5883l.md
Added caution to update_interval description about potential I2C errors at very short intervals; wording-only change, no functional modifications.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ❓ Inconclusive The linked issue [#10769] asks for identifying and fixing I2C NACK errors or providing a documentation/workaround; this PR implements a documentation mitigation by adding a caution about very short update_interval values but does not include any code changes to address the reported root cause, so it only partially satisfies the issue objectives. Because the issue's primary goal appears to be a root-cause fix (though a docs/workaround was suggested), it is unclear from the PR alone whether documentation-only treatment is sufficient to resolve the issue. Clarify whether maintainers accept documentation as an acceptable resolution for #10769; if so, state in the PR that this is a workaround, explicitly link the issue, and consider adding recommended safe update_interval examples or test notes, otherwise provide or link the corresponding esphome code fix PR that addresses the I2C NACKs.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title "[qmc5883l] Add note about I2C errors" concisely and accurately summarizes the primary change — a documentation update for the QMC5883L sensor to add a caution about I2C errors and short update intervals, which matches the raw_summary showing only wording changes to content/components/sensor/qmc5883l.md. It is specific, clear, and appropriate for repository history scanning.
Out of Scope Changes Check ✅ Passed The PR only modifies the QMC5883L documentation file and the change directly relates to the linked issue's suggested documentation/workaround, so there are no out-of-scope or unrelated code changes present in this pull request.
Description Check ✅ Passed The PR description states the change is a note about I2C errors and short update intervals and references the related issue #10769, so it is directly related to the documented change and not off-topic. The level of detail is sufficient for a documentation-only update.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
content/components/sensor/qmc5883l.md (4)

63-64: Make the warning actionable and consistent (I²C vs I2C).

Clarify what "too short" means in practice and suggest mitigation steps. Also use consistent “I²C” styling used elsewhere in this page.

-**update_interval** (*Optional*, [Time](#config-time)): The interval to check the sensor. Note that if the
-  interval is too short it may result in I2C errors. Defaults to `60s`.
+**update_interval** (*Optional*, [Time](#config-time)): How often to read the sensor. Very short intervals may lead to I²C transaction errors (NACK) on some modules, especially when combined with high oversampling. If you encounter I²C errors, increase the interval and/or lower oversampling. Defaults to `60s`. See also the linked issue for context.

12-15: Fix typos and tighten wording.

Correct spelling and phrasing; keep tone consistent with ESPHome docs.

-ESPHome. This sensor is very simular to the [HMC5883L](#hmc5883l) sensor and is oftern found
-as a knock off replacement. The QMC5883L sensor performs on par to the HMC5883L sensor,
-though the configuration differs. The [I²C Bus](#i2c) is required to be set up in your
-configuration for this sensor to work.
+ESPHome. This sensor is very similar to the [HMC5883L](#hmc5883l) sensor and is often found
+as a knock‑off replacement. The QMC5883L sensor performs on par with the HMC5883L sensor,
+though the configuration differs. The [I²C Bus](#i2c) must be set up in your
+configuration for this sensor to work.

43-44: Capitalize “I²C” consistently.

Match casing used elsewhere on this page.

-- **address** (*Optional*, int): Manually specify the I²c address of the sensor. Defaults to `0x0D`.
+- **address** (*Optional*, int): Manually specify the I²C address of the sensor. Defaults to `0x0D`.

77-79: Fix spelling and clarify oversampling text.

“adverage” → “average”; avoid ambiguous “x samples”.

-By default, the QMC5883L sensor measures each value 512 times when requesting a new value. You can, however,
-configure this amount. The result is the sensor will take the adverage of the x samples. Possible sampling values:
+By default, the QMC5883L sensor oversamples 512 times per reading. You can configure this.
+The sensor reports the average of those samples. Possible sampling values:
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bafffc3 and d20492d.

📒 Files selected for processing (1)
  • content/components/sensor/qmc5883l.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/qmc5883l.md

Copy link

netlify bot commented Sep 22, 2025

Deploy Preview for esphome ready!

Name Link
🔨 Latest commit d20492d
🔍 Latest deploy log https://app.netlify.com/projects/esphome/deploys/68d16b784a66aa00089debea
😎 Deploy Preview https://deploy-preview-5389--esphome.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mortification77
Copy link

FYI... I will be submitting a PR to update the QMC5883L Component to allow for the MAXIMUM DATA RATE that the device can handle per the Datasheet (200Hz).

Adding "drdy_pin" and "data_rate" config options that allow for the maximum data rate, and it is working well so far...

I need to "button up" some logging issues and I'll create the PR (probably this weekend)!

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

Successfully merging this pull request may close these issues.

QMC5883L I2C NACK Issues Since 2025.8
2 participants