Skip to content
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

Fix accuracy of grid_frequency #201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lowar
Copy link
Contributor

@lowar lowar commented Oct 27, 2024

  • documentation says 0.1Hz but its actually 0.01Hz

 - documentation says 0.1Hz but its actually 0.01Hz
@bohdan-s
Copy link
Owner

Hi. This one actually changes if using http or modbus methods. Really annoying. Not sure how to resolve that cleanly.

@lowar
Copy link
Contributor Author

lowar commented Oct 28, 2024

Ah, didn't know that. The last update to the WiNet-S module made the websocket connection unusable, so I can't test it at the moment.

Maybe one could replace the per register accuracy with a dict of the connection type? So instead of

  accuracy: 0.1

you would write

  accuracy:
    http: 0.1
    modbus: 0.01
    sungrow: 0.1

This simple change to the SubgrowClient will accomplish that. This way the client will work with the simple and the extended notation:

diff --git a/SungrowClient/SungrowClient.py b/SungrowClient/SungrowClient.py
index 867ebdb..41e5b33 100644
--- a/SungrowClient/SungrowClient.py
+++ b/SungrowClient/SungrowClient.py
@@ -279,7 +279,10 @@ class SungrowClient():
                             register_value = default
 
                     if register.get('accuracy'):
-                        register_value = round(register_value * register.get('accuracy'), 2)
+                        accuracy_value = register.get('accuracy')
+                        if isinstance(accuracy_value, dict):
+                          accuracy_value = accuracy_value.get(self.inverter_config['connection'])
+                        register_value = round(register_value * accuracy_value, 2)
 
                     # Set the final register value with adjustments above included 
                     self.latest_scrape[register_name] = register_value

Should I open a PR?

@michbeck100
Copy link
Collaborator

I would say the simple format should be default for all registers where accuracy is the same for all channels. So we should support both formats.

@michbeck100
Copy link
Collaborator

Just had a better look at your proposal. LGTM. Please create a PR

@lowar
Copy link
Contributor Author

lowar commented Oct 28, 2024

Done. bohdan-s/SungrowClient#12

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.

3 participants