Skip to content

Conversation

@Maelstrom96
Copy link
Owner

@Maelstrom96 Maelstrom96 commented Dec 4, 2021

What does this implement/fix?

Add support for the Emporia Vue 2.

The documentation is not quite ready yet, but we're pushing this out early so we can get some feedback on the code and implement changes if needed.

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

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

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266

Example entry for config.yaml:

# Example config.yaml
i2c:
  sda: 21
  scl: 22
  scan: false
  frequency: 200kHz
  id: i2c_a
  
sensor:
  - platform: emporia_vue
    i2c_id: i2c_a
    phases:
      - id: phase_a
        input: BLACK
        voltage: 
          name: "Phase A Voltage"
      - id: phase_c
        input: RED
        voltage: 
          name: "Phase C Voltage"
    ct_clamps:
      - name: "A"
        phase_id: phase_a
        input: "A"
      - name: "C"
        phase_id: phase_c
        input: "C"
      - name: "1"
        phase_id: phase_a
        input: "1"
      - name: "2"
        phase_id: phase_a
        input: "2"
      - name: "3"
        phase_id: phase_a
        input: "3"

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:

@Maelstrom96 Maelstrom96 closed this Dec 4, 2021
@Maelstrom96 Maelstrom96 reopened this Dec 4, 2021
@Maelstrom96 Maelstrom96 changed the title Add emporia vue2 Add new component - Emporia Vue Dec 6, 2021
Copy link

@krconv krconv left a comment

Choose a reason for hiding this comment

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

PR here to address these: #6; will also need to update the example config in docs

#include "esphome/components/i2c/i2c.h"
#include "esphome/components/sensor/sensor.h"

#ifdef USING_OTA_COMPONENT
Copy link

@krconv krconv Dec 6, 2021

Choose a reason for hiding this comment

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

I think that the FreeRTOS task is more hassle than it's worth, and will make this harder to maintain and debug in the long run. I think using a more traditional loop/polling approach is easier to understand and won't require the FreeRTOS dependency, global variable, nor specialized OTA setup.
The drawback is that the I2C read takes a while, ~23ms from my testing, which will slow down the loop() by that much every ~0.25s. Considering this hardware is specialized, I think that is ok.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure I understand why you say that it's more hassle than it's worth. It's already implemented and it's working as intended.

The main problem I was trying to alleviate with the task is missing readings. Using loop() or update() has no guarantee with the execution time. I was able to see this first hand when testing yesterday when loop() function of the emporia_vue component was not running fast enough when putting the logger option to VERY_VERBOSE, missing multiple sensor readings. That means that the other components could slow the main sensor loop enough that we would be missing power readings.

Right now, I'm only writing a single sensor reading to the queue, but my goal is to eventually add a way to handle a slow loop() and average all the sensor readings in the queue before publishing them, allowing total_daily_energy to still have a very accurate reading.

Copy link

Choose a reason for hiding this comment

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

Do you have a config where it's missing readings? I didn't realize that was a concern, I'm glad to test it out too (I have my Vue set up with 18 CT clamp and daily energy sensors, which I'm thinking is close to the most stress the loop() will have)

),
}
),
cv.Required(CONF_CT_CLAMPS): cv.ensure_list(SCHEMA_CT_CLAMP),
Copy link

Choose a reason for hiding this comment

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

I think nesting this sensor as a sub-object under the clamps config will help with future compatibility. We don't currently give a way to use the current sensor data, which I think we will eventually want to support. Nesting this sensor would help ensure we don't need to make a breaking change when we do add that data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants