thegridelectric / gw-scada-spaceheat-python

GridWorks SCADA for space heating
MIT License
5 stars 2 forks source link

Jm/emon #106

Closed jessicamillar closed 2 years ago

jessicamillar commented 2 years ago

This is a code branch I started when we were installing the emon Pi as a monitoring system up in Freedom ME. I've just gotten around to getting it tested & integrated.

The whole concept of the power meter needs to be cleaned up. Part of this will get cleaned up with a coherent and safe mechanism for settings data (i.e. selecting the driver for the power meter, and all the downstream ramifications that has, which could include how the SCADA calculates power before reporting it in a gs.pwr message.) But there are also at least two confounding classification issues

a) Multiple Power Meters There can be two multipurpose sensors which are both in the ActorClass PowerMeter but only one of them can be the meter that creates the gs.pwr messages etc (i.e. is acting as the revenue grade meter for all its electric metering data). This is the Atomic nature of an AtomicTNode - the power meter is indivisible. So we need to create a mechanism for identifying the Primary PowerMeter. This is where the concept of the ShNodeRole (which should be called SpaceHeatNodeRole) is helpful. The SpaceheatNodes will not share the same ShNodeRole (although the enum needs to be expanded to handle this). It is up for grabs whether they share the same ActorClass - seems like we might as well. It may be that the PowerMeter ActorClass can be generalized to be a MultipurposeSensor ActorClass - to mirror the SimpleSensor.

b) Multiple CTs per Power Meter The concept of multiple CurrentTransformers, with each CT pointing to the SpaceHeat node that it is measuring. This is the code update that I accidentally deleted. Its a pretty big upgrade but we will (probably) not need it all that soon. We have the first issue (a above) as soon as I can figure out how the modbus comms to the monitoring eGuage meter works.

Both of these intermingle with the settings issue. Once the settings are in a state we are both happy with, we'll want to address a) and b) above in separate PRs.

codecov[bot] commented 2 years ago

Codecov Report

Merging #106 (174e396) into main (1e4459d) will decrease coverage by 0.44%. The diff coverage is 73.71%.

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
- Coverage   94.38%   93.93%   -0.45%     
==========================================
  Files         216      216              
  Lines       10515    10577      +62     
==========================================
+ Hits         9925     9936      +11     
- Misses        590      641      +51     
Impacted Files Coverage Δ
gw_spaceheat/actors/scada.py 96.12% <ø> (-0.05%) :arrow_down:
...wer_meter/gridworks_sim_pm1__power_meter_driver.py 89.47% <ø> (-1.01%) :arrow_down:
...paceheat/schema/enums/make_model/make_model_map.py 86.36% <ø> (ø)
test/test_config.py 100.00% <ø> (ø)
...wer_meter/openenergy_emonpi__power_meter_driver.py 37.25% <37.25%> (ø)
..._spaceheat/data_classes/cacs/electric_meter_cac.py 80.00% <50.00%> (-20.00%) :arrow_down:
gw_spaceheat/actors2/nodes.py 71.96% <75.00%> (-9.29%) :arrow_down:
gw_spaceheat/actors/power_meter.py 95.95% <92.59%> (-0.80%) :arrow_down:
gw_spaceheat/actors/actor_base.py 84.49% <100.00%> (-0.88%) :arrow_down:
gw_spaceheat/config.py 100.00% <100.00%> (ø)
... and 8 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jessicamillar commented 2 years ago

The main hit here is that we don't have a framework for testing drivers, and mostly what I did here was add a driver.

I did also did reduce the coverage of actors2/nodes.py. Part of this (which I fixed) had to do with pulling the settings/configuration info about the power meter driver out of the actor base class where it really did not feel like it belonged, and cramming it (for the time being) back into the power_meter actor class. As a result I added power_meter_node (which has the required info without requiring a driver choice) to nodes.py.