Skip to content

Conversation

johnzielke
Copy link

Add info on how multiple meters can be read using a single HardwareSerial UART.

Description:

Related issue (if applicable): fixes

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.

Add info on how multiple meters can be read using a single HardwareSerial UART.
@Copilot Copilot AI review requested due to automatic review settings September 8, 2025 14:08
@esphome esphome bot added the current label Sep 8, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds documentation for multiplexing functionality to the SML component, enabling users to read data from multiple meters using a single UART on controllers with limited UART availability.

  • Adds a new section explaining how to multiplex between multiple SML meters
  • Provides a complete YAML configuration example showing UART multiplexing setup
  • Documents the use of the on_data trigger with lambda code to switch between GPIO pins

Comment on lines +236 to +237
- lambda: |-
std::vector<gpio_num_t> multiplex_pins = {::GPIO_NUM_17,::GPIO_NUM_19};
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The lambda code uses ESP-IDF specific gpio_num_t and GPIO_NUM_* constants without explaining this is ESP-specific. Consider adding a note that this example is for ESP32/ESP8266 platforms or provide a more platform-agnostic example.

Copilot uses AI. Check for mistakes.

static size_t current_index = 0;
current_index = (current_index + 1) % multiplex_pins.size();
gpio_num_t new_rx_pin = multiplex_pins[current_index];
id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The load_settings(true) call and its parameter are not explained. Consider adding a comment explaining what this does and why the true parameter is necessary for the multiplexing to work properly.

Suggested change
id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
// Reload the UART settings after changing the RX pin.
// The 'true' parameter forces the reload, which is necessary for multiplexing to work properly.

Copilot uses AI. Check for mistakes.

Copy link

netlify bot commented Sep 8, 2025

Deploy Preview for esphome ready!

Name Link
🔨 Latest commit a7e1c3a
🔍 Latest deploy log https://app.netlify.com/projects/esphome/deploys/68bee35b1c95620008875baa
😎 Deploy Preview https://deploy-preview-5341--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.

Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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: 1

🧹 Nitpick comments (3)
content/components/sml.md (3)

217-218: Clarify ESP32 UART count to avoid confusion.

ESP32 has three UART controllers (typically two free depending on board/usage). Suggest rephrasing to “more than the available UARTs on your board (ESP32 typically provides 3 UARTs).”


219-221: Tighten wording.

“An example on how to do this is this:” → “An example of how to do this:”

- An example on how to do this is this:
+ An example of how to do this:

236-243: Request compile-time validation of APIs used in lambda.

Please confirm that:

  • rx_pin accepts id and exposes set_pin(...)
  • UART component exposes load_settings(true) in current ESPHome
    If not, we should update the example to the supported API.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4512fb and a7e1c3a.

📒 Files selected for processing (1)
  • content/components/sml.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

  • Do not generate or add any sequence diagrams

Files:

  • content/components/sml.md

Comment on lines +222 to +254
```yaml
uart:
- baud_rate: 9600
data_bits: 8
rx_pin:
number: GPIO17 # Set to the first of the GPIO pins in multiplex_pins
id: uart_multiplex_rx_pin
stop_bits: 1
rx_buffer_size: 512
id: uart_multiplexed
sml:
- id: sml_multiplexed
uart_id: uart_multiplexed
on_data:
- lambda: |-
std::vector<gpio_num_t> multiplex_pins = {::GPIO_NUM_17,::GPIO_NUM_19};
static size_t current_index = 0;
current_index = (current_index + 1) % multiplex_pins.size();
gpio_num_t new_rx_pin = multiplex_pins[current_index];
id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
id(uart_multiplexed).load_settings(true);
sensor:
- platform: sml
name: "Solar Roof"
sml_id: sml_multiplexed
server_id: "12345ab" # IMPORTANT! Set the correct server id
obis_code: "1-0:2.8.0"
- platform: sml
name: "Solar Carport"
sml_id: sml_multiplexed
server_id: "67890cd" # IMPORTANT! Set the correct server id
obis_code: "1-0:2.8.0"
```
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix YAML indentation (invalid as written) and guard pin switch on valid frames.

  • The list items under uart:, sml:, and sensor: must be indented.
  • Only rotate RX after a valid SML frame to reduce missed reads.
  • Use realistic server_id examples (consistent length/format with earlier docs).
  • Consider adding a short note that the gpio_num_t example is ESP32/ESP-IDF–specific.
 ```yaml
-uart:
-- baud_rate: 9600
-  data_bits: 8
-  rx_pin:
-    number: GPIO17 # Set to the first of the GPIO pins in multiplex_pins
-    id: uart_multiplex_rx_pin
-  stop_bits: 1
-  rx_buffer_size: 512
-  id: uart_multiplexed
+uart:
+  - baud_rate: 9600
+    data_bits: 8
+    rx_pin:
+      number: GPIO17  # Set to the first of the GPIO pins in multiplex_pins
+      id: uart_multiplex_rx_pin
+    stop_bits: 1
+    rx_buffer_size: 512
+    id: uart_multiplexed
 sml:
-- id: sml_multiplexed
-  uart_id: uart_multiplexed
-  on_data:
-    - lambda: |-
-        std::vector<gpio_num_t> multiplex_pins = {::GPIO_NUM_17,::GPIO_NUM_19};
-        static size_t current_index = 0;
-        current_index = (current_index + 1) % multiplex_pins.size();
-        gpio_num_t new_rx_pin = multiplex_pins[current_index];
-        id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
-        id(uart_multiplexed).load_settings(true);
+  - id: sml_multiplexed
+    uart_id: uart_multiplexed
+    on_data:
+      - lambda: |-
+          // NOTE: ESP32-only example using ESP-IDF types (gpio_num_t).
+          std::vector<gpio_num_t> multiplex_pins = {::GPIO_NUM_17, ::GPIO_NUM_19};
+          if (valid) {
+            static size_t current_index = 0;
+            current_index = (current_index + 1) % multiplex_pins.size();
+            gpio_num_t new_rx_pin = multiplex_pins[current_index];
+            id(uart_multiplex_rx_pin).set_pin(new_rx_pin);
+            id(uart_multiplexed).load_settings(true);
+          }
 sensor:
-- platform: sml
-  name: "Solar Roof"
-  sml_id: sml_multiplexed
-  server_id: "12345ab" # IMPORTANT! Set the correct server id
-  obis_code: "1-0:2.8.0"
-- platform: sml
-  name: "Solar Carport"
-  sml_id: sml_multiplexed
-  server_id: "67890cd" # IMPORTANT! Set the correct server id
-  obis_code: "1-0:2.8.0"
+  - platform: sml
+    name: "Solar Roof"
+    sml_id: sml_multiplexed
+    server_id: "0123456789abcdef"  # Set your meter's server_id
+    obis_code: "1-0:2.8.0"
+  - platform: sml
+    name: "Solar Carport"
+    sml_id: sml_multiplexed
+    server_id: "fedcba9876543210"  # Set your meter's server_id
+    obis_code: "1-0:2.8.0"
🤖 Prompt for AI Agents
In content/components/sml.md around lines 222–254, fix the invalid YAML and
unsafe pin rotation: indent the list items under uart:, sml:, and sensor: so
each block is a proper list element (e.g., "- baud_rate..." under uart:), change
the on_data lambda to only rotate the RX pin after a validated SML frame (wrap
the current_index increment, set_pin and load_settings calls in an if(valid)
guard or equivalent), replace the example server_id values with realistic-length
hex strings consistent with the docs, and add a brief inline note that
gpio_num_t/::GPIO_NUM_* usage is ESP32/ESP-IDF specific.

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.

1 participant