Add Victron inverter #202

mduffin95 · 2024-09-20T22:15:09Z

Pull Request

Description

Adds support for Victron inverters.

How Has This Been Tested?

I've got access to a Victron system so I've been able to test it against their API. I've also included some response data from their API and used it to write a small test.

Checklist:

My code follows OCF's coding style guidelines
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked my code and corrected any misspellings

# Conflicts: # quartz_solar_forecast/inverters/enphase.py # quartz_solar_forecast/pydantic_models.py

This reverts commit f9f3c5f.

This reverts commit 94a9e70.

pyproject.toml

peterdudfield · 2024-09-26T17:45:36Z

hi @mduffin95

Thanks so much for this. Really appreciate you putting in the effort to put this in.
@zakwatts or @aryanbhosale do you fancy reviewing this?

Thanks

aryanbhosale · 2024-09-26T17:58:06Z

hi @mduffin95

Thanks so much for this. Really appreciate you putting in the effort to put this in. @zakwatts or @aryanbhosale do you fancy reviewing this?

Thanks

LGTM!
@mduffin95 would be nice if you could update the .env.example file from this PR

mduffin95 · 2024-10-13T20:08:55Z

Sorry for the late response, I've been away.

@aryanbhosale I've updated the pr with your suggested change

aryanbhosale · 2024-10-14T07:16:42Z

Sorry for the late response, I've been away.

@aryanbhosale I've updated the pr with your suggested change

Thank you, @peterdudfield i guess this could be merged now

peterdudfield · 2024-10-14T07:23:23Z

Sorry for the late response, I've been away.
@aryanbhosale I've updated the pr with your suggested change

Thank you, @peterdudfield i guess this could be merged now

Thanks, yea just one comment, we are trying to work out, about this repo - https://github.com/mduffin95/vrm-api-python-client

aryanbhosale · 2024-10-14T07:27:27Z

Sorry for the late response, I've been away.
@aryanbhosale I've updated the pr with your suggested change

Thank you, @peterdudfield i guess this could be merged now

Thanks, yea just one comment, we are trying to work out, about this repo - https://github.com/mduffin95/vrm-api-python-client

Yea i think opening an Issue and a PR in their original repo is a good idea, but i doubt theyll respond since the last activity was an issue that was opened 2 years back

peterdudfield · 2024-10-14T07:40:53Z

Sorry for the late response, I've been away.
@aryanbhosale I've updated the pr with your suggested change

Thank you, @peterdudfield i guess this could be merged now

Thanks, yea just one comment, we are trying to work out, about this repo - https://github.com/mduffin95/vrm-api-python-client

Yea i think opening an Issue and a PR in their original repo is a good idea, but i doubt theyll respond since the last activity was an issue that was opened 2 years back

Hm,yea, and it does say its not maintained. maybe some different options

Create PR into orginal repo from https://github.com/mduffin95/vrm-api-python-client
Create a vrim-api-python-client in OCF repos and publish to pypi
Pull in relevant code to this repo, perhaps we dont actually need that much

I would defiantly do 1. so other people can see the improvements. Im not sure about 2. or 3., probably depends how much code is needed for 3.

I think separately we should put this a an optional requirements in the pyproject.toml as most people dont need to install this.

mduffin95 · 2024-10-19T15:09:00Z

@peterdudfield @aryanbhosale

The original repo hasn't accepted any new PRs for years so I don't think 1 will work. IMO 2 is a better option than 3 for a couple of reasons:

It allows others to use a working python client for the Victron API for other purposes
It minimises the amount of vendor-specific code within this repo

I personally think it's better to reduce the scope of this repo to not include too much low-level API connectivity code. It keeps this repo clearly focussed on forecasting.

I think separately we should put this a an optional requirements in the pyproject.toml as most people dont need to install this.

☝️ I can make this change now.

aryanbhosale · 2024-10-19T15:24:05Z

@peterdudfield @aryanbhosale

The original repo hasn't accepted any new PRs for years so I don't think 1 will work. IMO 2 is a better option than 3 for a couple of reasons:

It allows others to use a working python client for the Victron API for other purposes

It minimises the amount of vendor-specific code within this repo

I personally think it's better to reduce the scope of this repo to not include too much low-level API connectivity code. It keeps this repo clearly focussed on forecasting.

I think separately we should put this a an optional requirements in the pyproject.toml as most people dont need to install this.

☝️ I can make this change now.

Sounds good to me

codecov · 2024-10-19T15:40:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.28%. Comparing base (5093843) to head (ec2da9d).
Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
+ Coverage   61.76%   62.28%   +0.51%     
==========================================
  Files          25       25              
  Lines        1020     1034      +14     
==========================================
+ Hits          630      644      +14     
  Misses        390      390

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

peterdudfield · 2024-10-20T17:19:41Z

Sounds. I'll create a new repo and @mduffin95 I'll make you to admin of it. I'm happy to do the pypi project things if you get the code ready

peterdudfield · 2024-10-20T17:27:40Z

Sounds. I'll create a new repo and @mduffin95 I'll make you to admin of it. I'm happy to do the pypi project things if you get the code ready

Its created here https://github.com/openclimatefix/vrm-api-python-client. @mduffin95 ive invited you to be an admin on this repo. Do you want to do any changes, and I can then sort the pypi stuff out

peterdudfield · 2024-10-23T12:31:41Z

Great stuff, thanks @mduffin95 for this. I miht merge into a development branch, to make sure all tests work e.t.c

* Add Victron inverter (#202) * wip * wip * solarman * update token * update inverters * undo some changes * add docstring * mock inverter docstring * remove dotenv * update givenergy * enphase * try to fix tests running twice * delete event * pr comments * revert workflow changes * split inverters into separate modules * import * Revert "import" This reverts commit f9f3c5f. * Revert "split inverters into separate modules" This reverts commit 94a9e70. * add pydantic_settings * set config within settings classes * add dependency * add victron * use named argument * start and end times * add a line to the documentation * dependencies * add test * add test * update script * add username and password to example env variable file * move vrmapi dependency to optional-dependencies * comment * update to use ocf_vrmapi * use issue/pip-all branch * add all to project.optional-dep * update import --------- Co-authored-by: Matthew Duffin <[email protected]>

mduffin95 added 30 commits August 14, 2024 00:02

wip

08194c9

wip

ca2370c

solarman

02bcdd2

update token

60c621e

update inverters

abca134

undo some changes

c610d39

add docstring

0bae5e9

mock inverter docstring

95d2d74

remove dotenv

effb3d6

Merge remote-tracking branch 'upstream/main' into topic/inverter-models

6038636

# Conflicts: # quartz_solar_forecast/inverters/enphase.py # quartz_solar_forecast/pydantic_models.py

update givenergy

a8449e6

enphase

fb3240d

try to fix tests running twice

f970b36

delete event

c925f87

pr comments

4dd6c3c

revert workflow changes

13f6ee4

split inverters into separate modules

94a9e70

import

f9f3c5f

Revert "import"

b2dabee

This reverts commit f9f3c5f.

Revert "split inverters into separate modules"

9cf1662

This reverts commit 94a9e70.

add pydantic_settings

3975f34

set config within settings classes

68f7045

add dependency

1b556eb

add victron

a7f7b39

use named argument

13fc2c5

start and end times

57d930c

add a line to the documentation

7545ef9

Merge branch 'main' into topic/victron

1664383

dependencies

71785a3

add test

e976f71

add test

52d99b8

mduffin95 commented Sep 20, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

mduffin95 changed the title ~~Topic/victron~~ Add Victron inverter Sep 20, 2024

update script

b26d650

mduffin95 marked this pull request as ready for review September 20, 2024 22:26

mduffin95 added 2 commits October 13, 2024 21:02

Merge remote-tracking branch 'upstream/main' into topic/victron

0d1d6d4

add username and password to example env variable file

e91944f

mduffin95 added 2 commits October 19, 2024 16:24

move vrmapi dependency to optional-dependencies

24881d8

comment

ec2da9d

mduffin95 requested a review from peterdudfield October 19, 2024 15:36

update to use ocf_vrmapi

0752ea9

peterdudfield changed the base branch from main to development October 25, 2024 14:11

peterdudfield merged commit 3f60fb7 into openclimatefix:development Oct 25, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Victron inverter #202

Add Victron inverter #202

mduffin95 commented Sep 20, 2024 •

edited

Loading

peterdudfield commented Sep 26, 2024

aryanbhosale commented Sep 26, 2024

mduffin95 commented Oct 13, 2024

aryanbhosale commented Oct 14, 2024 •

edited

Loading

peterdudfield commented Oct 14, 2024

aryanbhosale commented Oct 14, 2024

peterdudfield commented Oct 14, 2024

mduffin95 commented Oct 19, 2024 •

edited

Loading

aryanbhosale commented Oct 19, 2024

codecov bot commented Oct 19, 2024

peterdudfield commented Oct 20, 2024

peterdudfield commented Oct 20, 2024

peterdudfield commented Oct 23, 2024

Add Victron inverter #202

Add Victron inverter #202

Conversation

mduffin95 commented Sep 20, 2024 • edited Loading

Pull Request

Description

How Has This Been Tested?

Checklist:

peterdudfield commented Sep 26, 2024

aryanbhosale commented Sep 26, 2024

mduffin95 commented Oct 13, 2024

aryanbhosale commented Oct 14, 2024 • edited Loading

peterdudfield commented Oct 14, 2024

aryanbhosale commented Oct 14, 2024

peterdudfield commented Oct 14, 2024

mduffin95 commented Oct 19, 2024 • edited Loading

aryanbhosale commented Oct 19, 2024

codecov bot commented Oct 19, 2024

Codecov Report

peterdudfield commented Oct 20, 2024

peterdudfield commented Oct 20, 2024

peterdudfield commented Oct 23, 2024

mduffin95 commented Sep 20, 2024 •

edited

Loading

aryanbhosale commented Oct 14, 2024 •

edited

Loading

mduffin95 commented Oct 19, 2024 •

edited

Loading