Move pulse meter flash trigger to total sensor #690
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant Sensor as sensor_total_energy
participant LED as led_red
Sensor->>LED: Raw value received
LED->>LED: Flash for 100ms
The sequence diagram illustrates the simplified event handling: when a raw value is received from the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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)
components/pulse_meter.yaml (1)
74-78: Consider documenting flash timing behaviorWhile the 100ms flash timing is well-chosen for typical usage (distinct flashes below 36kW), consider adding a comment in the configuration to document:
- The relationship between power consumption and flash overlap
- The rationale for choosing 100ms (helps future maintenance)
Example comment to add above the on_raw_value section:
# Flash timing: 100ms provides distinct flashes for loads below 36kW # For meters with 1000 imp/kWh, flashes may overlap at higher loads
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/pulse_meter.yaml(1 hunks)
🔇 Additional comments (2)
components/pulse_meter.yaml (2)
Line range hint 31-73: Sensor configuration maintains proper metrics and calculations
The pulse meter and total energy sensor configurations are well structured with:
- Appropriate state_class and device_class assignments
- Correct unit conversions through lambda filters
- Proper handling of pulse rate calculations
74-78: Implementation aligns well with PR objectives
The new flash trigger implementation using flash_length: 100ms is a good solution that:
- Simplifies the code by using an atomic operation
- Reduces the risk of flash interference during high pulse rates
- Provides distinct flashes for loads below 36kW as required
Let's verify the flash_length parameter support:
In #135 it was determined that when the Pulse Meter reads the same duration between subsequent pulses, the sensor is not updated and
on_raw_valueis never triggered so the LED never flashes. This becomes increasingly likely during high usage as pulse durations become shorter.This PR moves the trigger to the Pulse Meter's Total sensor, which does get triggered for every pulse.
The
light.turn_onaction can also be called with aflash_lengthparameter, rather than usingturn_on,delay,turn_off.During high pulse rates a first pulse's
turn_offmay trigger between a later pulse'sturn_onandturn_off, resulting in inconsistent flash lengths, though I'm not sure if usingflash_lengthmight address this. With a meter that pulses for each Wh (1000 / kWh), 100ms flashes should be distinct for loads below 36 kW, while 200ms flashes would overlap at 18kW.Summary by CodeRabbit