-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Homewizard improvements #26938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Homewizard improvements #26938
Changes from 4 commits
22bdac9
88a586e
25c89ad
9bcd575
d53e202
4adaafc
50da2af
9ef4c83
99903e7
560824e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,22 +17,28 @@ type Connection struct { | |
| *request.Helper | ||
| uri string | ||
| usage string | ||
| phase int | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The entire single phase mapping is a bad idea, please remove.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But why is it a bad idea? Load limits apply to each phase and so the measurements should be counted to the right phase.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But why is it a bad idea? Load limits apply to each phase and so the measurements should be counted to the right phase.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We‘re not doing this for any device
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove or open fresh pr without |
||
| ProductType string | ||
| dataG util.Cacheable[DataResponse] | ||
| stateG util.Cacheable[StateResponse] | ||
| } | ||
|
|
||
| // NewConnection creates a homewizard connection | ||
| func NewConnection(uri string, usage string, cache time.Duration) (*Connection, error) { | ||
| func NewConnection(uri string, usage string, phase int, cache time.Duration) (*Connection, error) { | ||
| if uri == "" { | ||
| return nil, errors.New("missing uri") | ||
| } | ||
|
|
||
| if phase < 1 || phase > 3 { | ||
| return nil, errors.New("phase must be between 1 and 3") | ||
| } | ||
|
|
||
| log := util.NewLogger("homewizard") | ||
| c := &Connection{ | ||
| Helper: request.NewHelper(log), | ||
| uri: fmt.Sprintf("%s/api", util.DefaultScheme(strings.TrimRight(uri, "/"), "http")), | ||
| usage: usage, | ||
| phase: phase, | ||
| } | ||
|
|
||
| c.Client.Transport = request.NewTripper(log, transport.Insecure()) | ||
|
|
@@ -101,7 +107,7 @@ func (c *Connection) Enabled() (bool, error) { | |
| // CurrentPower implements the api.Meter interface | ||
| func (c *Connection) CurrentPower() (float64, error) { | ||
| res, err := c.dataG.Get() | ||
| if c.usage == "pv" { | ||
| if c.usage == "pv" || c.usage == "battery" { | ||
| return -res.ActivePowerW, err | ||
| } | ||
| return res.ActivePowerW, err | ||
|
|
@@ -110,16 +116,36 @@ func (c *Connection) CurrentPower() (float64, error) { | |
| // TotalEnergy implements the api.MeterEnergy interface | ||
| func (c *Connection) TotalEnergy() (float64, error) { | ||
| res, err := c.dataG.Get() | ||
| if c.usage == "pv" { | ||
| return res.TotalPowerExportT1kWh + res.TotalPowerExportT2kWh + res.TotalPowerExportT3kWh + res.TotalPowerExportT4kWh, err | ||
| if c.usage == "pv" || c.usage == "battery" { | ||
| return res.TotalPowerExportkWh, err | ||
| } | ||
| return res.TotalPowerImportT1kWh + res.TotalPowerImportT2kWh + res.TotalPowerImportT3kWh + res.TotalPowerImportT4kWh, err | ||
| return res.TotalPowerImportkWh, err | ||
| } | ||
|
|
||
| // Currents implements the api.PhaseCurrents interface | ||
| func (c *Connection) Currents() (float64, float64, float64, error) { | ||
| res, err := c.dataG.Get() | ||
| if c.usage == "pv" { | ||
|
|
||
| // Single-phase meters only have one current reading | ||
| if c.ProductType == "HWE-KWH1" || c.ProductType == "SDM230-wifi" { | ||
| current := res.ActiveCurrentA | ||
| if c.usage == "pv" || c.usage == "battery" { | ||
| current = -current | ||
| } | ||
|
|
||
| // Return current on configured phase | ||
| switch c.phase { | ||
| case 1: | ||
| return current, 0, 0, err | ||
| case 2: | ||
| return 0, current, 0, err | ||
| case 3: | ||
| return 0, 0, current, err | ||
| } | ||
| } | ||
|
|
||
| // Three-phase meters have separate current readings per phase | ||
| if c.usage == "pv" || c.usage == "battery" { | ||
| return -res.ActiveCurrentL1A, -res.ActiveCurrentL2A, -res.ActiveCurrentL3A, err | ||
| } | ||
| return res.ActiveCurrentL1A, res.ActiveCurrentL2A, res.ActiveCurrentL3A, err | ||
|
|
@@ -128,5 +154,22 @@ func (c *Connection) Currents() (float64, float64, float64, error) { | |
| // Voltages implements the api.PhaseVoltages interface | ||
| func (c *Connection) Voltages() (float64, float64, float64, error) { | ||
| res, err := c.dataG.Get() | ||
|
|
||
| // Single-phase meters only have one voltage reading | ||
| if c.ProductType == "HWE-KWH1" || c.ProductType == "SDM230-wifi" { | ||
| voltage := res.ActiveVoltageV | ||
|
|
||
| // Return voltage on configured phase | ||
| switch c.phase { | ||
| case 1: | ||
| return voltage, 0, 0, err | ||
| case 2: | ||
| return 0, voltage, 0, err | ||
| case 3: | ||
| return 0, 0, voltage, err | ||
| } | ||
| } | ||
|
|
||
| // Three-phase meters have separate voltage readings per phase | ||
| return res.ActiveVoltageL1V, res.ActiveVoltageL2V, res.ActiveVoltageL3V, err | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,19 +16,15 @@ type StateResponse struct { | |
| // DataResponse returns the most recent measurements from the HomeWizard device | ||
| // https://homewizard-energy-api.readthedocs.io/endpoints.html#state-api-v1-state | ||
| type DataResponse struct { | ||
| ActivePowerW float64 `json:"active_power_w"` | ||
| TotalPowerImportT1kWh float64 `json:"total_power_import_t1_kwh"` | ||
| TotalPowerImportT2kWh float64 `json:"total_power_import_t2_kwh"` | ||
| TotalPowerImportT3kWh float64 `json:"total_power_import_t3_kwh"` | ||
| TotalPowerImportT4kWh float64 `json:"total_power_import_t4_kwh"` | ||
| TotalPowerExportT1kWh float64 `json:"total_power_export_t1_kwh"` | ||
| TotalPowerExportT2kWh float64 `json:"total_power_export_t2_kwh"` | ||
| TotalPowerExportT3kWh float64 `json:"total_power_export_t3_kwh"` | ||
| TotalPowerExportT4kWh float64 `json:"total_power_export_t4_kwh"` | ||
| ActiveCurrentL1A float64 `json:"active_current_l1_a"` | ||
| ActiveCurrentL2A float64 `json:"active_current_l2_a"` | ||
| ActiveCurrentL3A float64 `json:"active_current_l3_a"` | ||
| ActiveVoltageL1V float64 `json:"active_voltage_l1_v"` | ||
| ActiveVoltageL2V float64 `json:"active_voltage_l2_v"` | ||
| ActiveVoltageL3V float64 `json:"active_voltage_l3_v"` | ||
| ActivePowerW float64 `json:"active_power_w"` | ||
| TotalPowerImportkWh float64 `json:"total_power_import_kwh"` | ||
| TotalPowerExportkWh float64 `json:"total_power_export_kwh"` | ||
| ActiveCurrentA float64 `json:"active_current_a"` | ||
| ActiveCurrentL1A float64 `json:"active_current_l1_a"` | ||
| ActiveCurrentL2A float64 `json:"active_current_l2_a"` | ||
| ActiveCurrentL3A float64 `json:"active_current_l3_a"` | ||
| ActiveVoltageV float64 `json:"active_voltage_v"` | ||
| ActiveVoltageL1V float64 `json:"active_voltage_l1_v"` | ||
| ActiveVoltageL2V float64 `json:"active_voltage_l2_v"` | ||
| ActiveVoltageL3V float64 `json:"active_voltage_l3_v"` | ||
|
Comment on lines
+23
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question (bug_risk): Switching from tariff-based totals to aggregate import/export fields may affect compatibility with existing devices/firmware. This change assumes |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,11 +38,11 @@ func TestUnmarshalKwhDataResponse(t *testing.T) { | |
| { | ||
| var res DataResponse | ||
| // https://www.homewizard.com/shop/wi-fi-kwh-meter-1-phase/ | ||
| jsonstr := `{"wifi_ssid": "My Wi-Fi","wifi_strength": 100,"total_power_import_t1_kwh": 30.511,"total_power_export_t1_kwh": 85.951,"active_power_w": 543,"active_power_l1_w": 28,"active_power_l2_w": 0,"active_power_l3_w": -181,"active_voltage_l1_v": 235.4,"active_voltage_l2_v": 235.8,"active_voltage_l3_v": 236.1,"active_current_l1_a": 1.19,"active_current_l2_a": 0.37,"active_current_l3_a": -0.93}` | ||
| jsonstr := `{"wifi_ssid": "My Wi-Fi","wifi_strength": 100,"total_power_import_kwh": 30.511,"total_power_export_kwh": 85.951,"total_power_import_t1_kwh": 30.511,"total_power_export_t1_kwh": 85.951,"active_power_w": 543,"active_power_l1_w": 28,"active_power_l2_w": 0,"active_power_l3_w": -181,"active_voltage_l1_v": 235.4,"active_voltage_l2_v": 235.8,"active_voltage_l3_v": 236.1,"active_current_l1_a": 1.19,"active_current_l2_a": 0.37,"active_current_l3_a": -0.93}` | ||
aritmeester marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| require.NoError(t, json.Unmarshal([]byte(jsonstr), &res)) | ||
|
|
||
| assert.Equal(t, float64(30.511), res.TotalPowerImportT1kWh+res.TotalPowerImportT2kWh+res.TotalPowerImportT3kWh+res.TotalPowerImportT4kWh) | ||
| assert.Equal(t, float64(85.951), res.TotalPowerExportT1kWh+res.TotalPowerExportT2kWh+res.TotalPowerExportT3kWh+res.TotalPowerExportT4kWh) | ||
| assert.Equal(t, float64(30.511), res.TotalPowerImportkWh) | ||
| assert.Equal(t, float64(85.951), res.TotalPowerExportkWh) | ||
|
Comment on lines
+44
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Assert new current/voltage aggregate fields in kWh data response Please also extend this test to assert the Suggested implementation: jsonstr := `{"wifi_ssid":"redacted","wifi_strength":78,"smr_version":50,"meter_model":"Landis + Gyr","unique_id":"redacted","active_tariff":2,"total_power_import_kwh":18664.997,"total_power_import_t1_kwh":10909.724,"total_power_import_t2_kwh":7755.273,"total_power_export_kwh":13823.608,"total_power_export_t1_kwh":4243.981,"total_power_export_t2_kwh":9579.627,"active_power_w":203.000,"active_power_l1_w":-21.000,"active_power_l2_w":57.000,"active_power_l3_w":168.000,"active_voltage_v":226.000,"active_voltage_l1_v":228.000,"active_voltage_l2_v":226.000,"active_voltage_l3_v":225.000,"active_current_a":1.091,"active_current_l1_a":-0.092,"active_current_l2_a":0.252,"active_current_l3_a":0.747,"voltage_sag_l1_count":12.000,"voltage_sag_l2_count":12.000,"voltage_sag_l3_count":19.000,"voltage_swell_l1_count":5055.000,"voltage_swell_l2_count":1950.000,"voltage_swell_l3_count":0.000,"any_power_fail_count":12.000,"long_power_fail_count":2.000,"total_gas_m3":5175.363,"gas_timestamp":241106093006,"gas_unique_id":"redacted","external":[{"unique_id":"redacted","type":"gas_meter","timestamp":241106093006,"value":5175.363,"unit":"m3"}]}` assert.Equal(t, float64(18664.997), res.TotalPowerImportkWh)
assert.Equal(t, float64(13823.608), res.TotalPowerExportkWh)
assert.Equal(t, float64(203), res.ActivePowerW)
assert.Equal(t, float64(226), res.ActiveVoltageV)
assert.Equal(t, float64(1.091), res.ActiveCurrentA)
assert.Equal(t, float64(228), res.ActiveVoltageL1V) |
||
| assert.Equal(t, float64(543), res.ActivePowerW) | ||
| assert.Equal(t, float64(235.4), res.ActiveVoltageL1V) | ||
| assert.Equal(t, float64(235.8), res.ActiveVoltageL2V) | ||
|
|
@@ -61,8 +61,8 @@ func TestUnmarshalP1DataResponse(t *testing.T) { | |
| jsonstr := `{"wifi_ssid":"redacted","wifi_strength":78,"smr_version":50,"meter_model":"Landis + Gyr","unique_id":"redacted","active_tariff":2,"total_power_import_kwh":18664.997,"total_power_import_t1_kwh":10909.724,"total_power_import_t2_kwh":7755.273,"total_power_export_kwh":13823.608,"total_power_export_t1_kwh":4243.981,"total_power_export_t2_kwh":9579.627,"active_power_w":203.000,"active_power_l1_w":-21.000,"active_power_l2_w":57.000,"active_power_l3_w":168.000,"active_voltage_l1_v":228.000,"active_voltage_l2_v":226.000,"active_voltage_l3_v":225.000,"active_current_a":1.091,"active_current_l1_a":-0.092,"active_current_l2_a":0.252,"active_current_l3_a":0.747,"voltage_sag_l1_count":12.000,"voltage_sag_l2_count":12.000,"voltage_sag_l3_count":19.000,"voltage_swell_l1_count":5055.000,"voltage_swell_l2_count":1950.000,"voltage_swell_l3_count":0.000,"any_power_fail_count":12.000,"long_power_fail_count":2.000,"total_gas_m3":5175.363,"gas_timestamp":241106093006,"gas_unique_id":"redacted","external":[{"unique_id":"redacted","type":"gas_meter","timestamp":241106093006,"value":5175.363,"unit":"m3"}]}` | ||
| require.NoError(t, json.Unmarshal([]byte(jsonstr), &res)) | ||
|
|
||
| assert.Equal(t, float64(18664.997), res.TotalPowerImportT1kWh+res.TotalPowerImportT2kWh+res.TotalPowerImportT3kWh+res.TotalPowerImportT4kWh) | ||
| assert.Equal(t, float64(13823.608), res.TotalPowerExportT1kWh+res.TotalPowerExportT2kWh+res.TotalPowerExportT3kWh+res.TotalPowerExportT4kWh) | ||
| assert.Equal(t, float64(18664.997), res.TotalPowerImportkWh) | ||
| assert.Equal(t, float64(13823.608), res.TotalPowerExportkWh) | ||
sourcery-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| assert.Equal(t, float64(203), res.ActivePowerW) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add tests for Since
This will guard against regressions in phase validation and defaulting during future refactors. |
||
| assert.Equal(t, float64(228), res.ActiveVoltageL1V) | ||
| assert.Equal(t, float64(226), res.ActiveVoltageL2V) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.