Skip to content

Conversation

lbbrhzn
Copy link
Owner

@lbbrhzn lbbrhzn commented Jul 21, 2021

Bugfix processing phase measurands. See #95

Bugfix processing phase measurands
@lbbrhzn lbbrhzn requested a review from drc38 July 21, 2021 20:36
@github-actions
Copy link

github-actions bot commented Jul 21, 2021

🎉 HACS repository validator action summary 🎉
✅ The repository is not archived
✅ manifest.json file exist
✅ All required keys are present in manifest.json
✅ hacs.json has the 'name' key set
✅ The repository has a description
⚪ Ignored check: brands
✅ The repository has topics
✅ The repository has issues enabled
✅ This day ends with an 'y' (Friday)
✅ README.md exists
✅ Requirements validation
✅ Python wheels
✅ HACS load check

@lbbrhzn
Copy link
Owner Author

lbbrhzn commented Jul 21, 2021

@drc38, why are positive voltages treated differently from negative?

           if metric in Measurand.voltage.value:
                sum = (
                    value[Phase.l1_n.value]
                    + value[Phase.l2_n.value]
                    + value[Phase.l3_n.value]
                )
                if sum > 0:
                    self._metrics[metric] = round(sum / 3, 1)
                else:
                    self._metrics[metric] = round(sum, 1)

@drc38
Copy link
Collaborator

drc38 commented Jul 21, 2021

@drc38, why are positive voltages treated differently from negative?

           if metric in Measurand.voltage.value:
                sum = (
                    value[Phase.l1_n.value]
                    + value[Phase.l2_n.value]
                    + value[Phase.l3_n.value]
                )
                if sum > 0:
                    self._metrics[metric] = round(sum / 3, 1)
                else:
                    self._metrics[metric] = round(sum, 1)

There should not be negative voltages, and the >0 was to protect against a div by 0 error.

@lbbrhzn
Copy link
Owner Author

lbbrhzn commented Jul 21, 2021

  if sum > 0:
                    self._metrics[metric] = round(sum / 3, 1)
                else:
                    self._metrics[metric] = round(sum, 1)

You’re never dividing by sum though. And if sum equals zero, the metric value will be zero.

Ok if I just change it to:

self._metrics[metric] = round(sum / 3, 1)

@drc38
Copy link
Collaborator

drc38 commented Jul 21, 2021

😄 good spotting, I was working too late. Please update

@drc38
Copy link
Collaborator

drc38 commented Jul 21, 2021

Will you also update prior loop to reference self._extra_attr?

Improve processing of voltage phases
@lbbrhzn
Copy link
Owner Author

lbbrhzn commented Jul 23, 2021

Will you also update prior loop to reference self._extra_attr?

I will keep that the same for now, but I do think there is some room improvement. We should aim to provide the one-phase equivalent voltage, current and power when three phases are reported. See here for some background info. Assuming the voltages Vx-n are balanced, we can simply add the currents to get the one-phase equivalent current. When line-to-line voltages are reported we should divide by sqrt(3) to get the line to neutral voltage. The line-to-neutral voltages are simply averaged.

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