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

Feature - null value handling #85

Merged
merged 15 commits into from
Jun 22, 2023
Merged

Feature - null value handling #85

merged 15 commits into from
Jun 22, 2023

Conversation

0x7878
Copy link
Collaborator

@0x7878 0x7878 commented May 8, 2023

untested on a real device but should resolve issue #72

Please review what I did and let's discuss if you don't like it. Furthermore, I introduce unit tests in our codebase and implement full coverage for the helpers.py which I also used to test this issue.

Copy link
Collaborator

@dsteinkopf dsteinkopf left a comment

Choose a reason for hiding this comment

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

Thank you very much.

dbus_service.py Outdated Show resolved Hide resolved
dbus_service.py Outdated Show resolved Hide resolved
helpers.py Outdated Show resolved Hide resolved
helpers.py Outdated Show resolved Hide resolved
helpers.py Outdated Show resolved Hide resolved
test_helpers.py Show resolved Hide resolved
dbus_service.py Outdated Show resolved Hide resolved
dbus_service.py Outdated Show resolved Hide resolved
@0x7878
Copy link
Collaborator Author

0x7878 commented May 14, 2023

Alright, I gave my best to refactor the codebase. I cannot test the code on an actual device until Wednesday. But maybe there are still some changes left :) Let's see

helpers.py Outdated Show resolved Hide resolved
helpers.py Outdated Show resolved Hide resolved
helpers.py Outdated Show resolved Hide resolved
test_helpers.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
test_helpers.py Show resolved Hide resolved
@0x7878 0x7878 requested a review from dsteinkopf May 23, 2023 11:45
@0x7878
Copy link
Collaborator Author

0x7878 commented May 23, 2023

Please review the changes. I've tested this version on my "dev device". Also, I fixed a typo in the config which will be a breaking change (magAge...) instead of (maxAge...) . Furthermore, I change the readConfig section to allow default values, reducing the config file havely. I mean there are only a few lines necessary now to run the service.

Copy link
Collaborator

@dsteinkopf dsteinkopf left a comment

Choose a reason for hiding this comment

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

Please also check the remaining open conversations from the former reviews above. (I tried to correctly close the finished ones and leave those open where there is still something to do. Please leave them open, I'll do that.)

README.md Show resolved Hide resolved
@0x7878 0x7878 requested a review from dsteinkopf June 21, 2023 09:11
@henne49 henne49 changed the title Feateure null value handling Feature - null value handling Jun 21, 2023
Small improvements, typos etc.
Copy link
Collaborator

@dsteinkopf dsteinkopf left a comment

Choose a reason for hiding this comment

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

I've added some corrections to the readme. If you think they are ok, feel free to merge.

@0x7878 0x7878 merged commit f94aa4c into main Jun 22, 2023
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.

Report soyo inverter status "null" als Werte für Power, Current, Volt ?
3 participants