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

Add support for SOFAR 2200TL #311

Merged
merged 6 commits into from
Jan 20, 2023

Conversation

darshkpatel
Copy link
Contributor

I tried using this library for my SOFAR inverter and noticed it was failing due to a space while parsing integer for the rated power substring it tries to read from the myDeviceArray string.

I have fixed that issue, updated the test to test this edge case too, and also updated the documentation adding support for SOFAR 2200TL🥂

Error Log for reference:

  File "/usr/local/lib/python3.10/site-packages/omnikinverter/omnikinverter.py", line 184, in inverter
    return Inverter.from_js(data)
  File "/usr/local/lib/python3.10/site-packages/omnikinverter/models.py", line 177, in from_js
    solar_rated_power=get_value(4),
  File "/usr/local/lib/python3.10/site-packages/omnikinverter/models.py", line 158, in get_value
    return int(matches[position])
ValueError: invalid literal for int() with base 10: '2 00'

@klaasnicolaas klaasnicolaas added the enhancement Enhancement of the code, not introducing new features. label Dec 23, 2022
@codecov
Copy link

codecov bot commented Dec 23, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (64ccafb) compared to base (5a5dcf7).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #311   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          324       324           
  Branches        55        55           
=========================================
  Hits           324       324           
Impacted Files Coverage Δ
omnikinverter/models.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@klaasnicolaas klaasnicolaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make a separate test with its own data fixture?

omnikinverter/models.py Outdated Show resolved Hide resolved
@klaasnicolaas klaasnicolaas added the in-progress Issue is currently being resolved by a developer. label Dec 23, 2022
@darshkpatel
Copy link
Contributor Author

Could you make a separate test with its own data fixture?

Done in b184a28

@klaasnicolaas klaasnicolaas merged commit 9d83c11 into klaasnicolaas:main Jan 20, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement of the code, not introducing new features. in-progress Issue is currently being resolved by a developer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants