New energy meter of type Hager ECR380D#3336
Conversation
This PR has been automatically marked as stale due to inactivityIf it is still relevant, please update the PR. Before updating, please review our Contribution Guidelines to ensure all required information is provided. |
sfeilmeier
left a comment
sfeilmeier
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Well done! Please see my note on the modbus tasks implemtation.
Also, if you could allow maintainers to edit your PR, I could do some final touches and formattings myself.
Thanks & Regards, Stefan
| */ | ||
| @Override | ||
| protected ModbusProtocol defineModbusProtocol() { | ||
| final ModbusProtocol modbusProtocol = new ModbusProtocol(this); |
There was a problem hiding this comment.
I think I understand your intention. I would still prefer using the same coding style as described in the Docs and used in other Modbus implementations for readability and maintainability
There was a problem hiding this comment.
Hello Stefan, first of all: Happy New Year!
Thanks a lot for comming back on my PR. Can you please provide me a link to the Docs you mentioned? In the meantime, I'll have a look at other Modbus implementations ;-)
I hope I used the correct flag to enable write access to the PR...
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @sfeilmeier, I should have seen this 🙈
I'll change my code as described in the instructions and other Modbus modules...
There was a problem hiding this comment.
Hello @sfeilmeier ,
I add code changes related to the Modbus configuration. Please see my latest commit. Please let me know if you find more inconsistencies.
Thanks & Regards,
Heinz
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3336 +/- ##
=============================================
+ Coverage 59.22% 59.37% +0.16%
Complexity 105 105
=============================================
Files 3040 3068 +28
Lines 132217 133081 +864
Branches 9756 9815 +59
=============================================
+ Hits 78293 79005 +712
- Misses 50976 51105 +129
- Partials 2948 2971 +23 🚀 New features to boost your workflow:
|
sfeilmeier
left a comment
sfeilmeier
left a comment
There was a problem hiding this comment.
Thank you for your contribution! 🚀
2 participants
For personal use, I created a new bundle for a Hager ECR380D energy meter. I'd like to provide my additions to the OpenEMS community.
I tried to follow the implementation examples, but I'd appreciate any review, since this is my first contribution to the OpenEMS community. The new bundle is up and running for several days and report data as expected (Power, Current, Voltage).
Any feedback, improvements are warmly welcome.