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

[mysolarpvdivert] Add support for batteries #30

Closed
wants to merge 1 commit into from

Conversation

sysrqio
Copy link

@sysrqio sysrqio commented Jun 28, 2017

Added support for battery feeds.
New optional feeds:

  • Discharged from Battery
  • Discharged from Battery in kWh
  • Percentage of the battery charging state

Note: The use feed needs to include house consumption and the divert (battery loading)
It was required to change the statustable a bit.

Attached you find pictures of the changed behaviour.

Using no optional data with:
{"My Solar Divert":{"app":"mysolarpvdivert","config":{"use":"5","solar":"11","use_kwh":"6","solar_kwh":"12","divert_kwh":"22","import_kwh":"8","divert":"21"}}}
image
Using new battery feature with:
{"My Solar Divert":{"app":"mysolarpvdivert","config":{"use":"45","solar":"11","divert":"21","export":"9","discharge":"19","use_kwh":"46","solar_kwh":"12","divert_kwh":"22","import_kwh":"8","export_kwh":"10","discharge_kwh":"20","charge_prc":"15"}}}
image

Additional information on the used feed data
use: House consumption + Battery loading
solar: PV generation
divert: Battery loading
discharge: Battery discharging
charge_prc: Battery percentage

@glynhudson
Copy link
Member

Wow, this looks amazing! Nice work. Have you got any comment @TrystanLea @pb66 @mattjgalloway ? Are you happy for this to be merged?

@pb66
Copy link
Contributor

pb66 commented Jul 1, 2017

Yes it does look impressive, although I do have concerns about whether this should (currently) be an extension to the "mysolarpvdivert" app or a separate "mysolarpvbattery" app. I suspect any proposed change in "use" to include "divert" would not be backwards compatible and therefore involve users changing their processing and there fore creating a new feed or re-purposing the existing feed to include divert, making the historical data incomparable to newly collected data.

I think it's great that the app can dynamically alter depending on the feeds found BUT I do not think this suits all use cases.

Many more users have DHW diverters and/or underfloor heating etc and only a small number have battery systems, I would hazard a guess and say just as many (if not more) have regular diversion to DHW/heating AND a battery system, as battery alone. in which case DHW and other diversion would be lumped in with charging the battery OR unaccounted for and just inflate the "use" without explanation.

If this was a separate "mysolarpvbattery" app then users can use the apps independently for either a battery only or a regular diversion system until an app can cater for both by reporting diversion and charging separately.

I think diverted consumption and diverted charging need handling slightly differently and therefore cannot be combined as a single diverted

In the same way "mysolarpvdivert" was added alongside "mysolarpv", this might be better placed, being added as "mysolarpvbattery" to avoid any complication and retain a choice, certainly until a "mysolarpvdivertbattery" app can cater for all.

The changes @TrystanLea has recently made to the apps module is to promote development of more apps so there seems little need to replace or remove an existing app. IMO it would be great to see this included but not replacing the current app (yet). I look forward to any comments from @mattjgalloway or @TrystanLea,

@sysrqio
Copy link
Author

sysrqio commented Jul 1, 2017

I was thinking about the exact same topic before starting. Is it better to create an own "app" and duplicate ~98% of the code (I did not count the lines) in mysolardivert or extend the mysolardivert? Since I had no idea was is preferred by this community I extended it.

Regarding my local setup to create a feed, which includes divert in use, I need to make clear that my emoncms installation was just a few hours old before submitting this change and I have a very limited knowledge about what a normal data feed looks like in emon.
All data used by me is pushed from the batteries api to the emoncms. This means that I have no data which comes from emon supported devices. I noticed that the values displayed for house consumption and house total where wrong for me. The numbers where correct after creating a feed which added divert to the house consumption. The main point on this is if use usually contains the divert in a normal setup. I am not using emon ways to gather the data.

In my local special setup I have following data:

house consumption = pure consumption by the house (excluding battery charging)
battery charging      = consumption while charging
...

Which makes me think that the user does not need to change anything in use if the data was correct before in a normal setup. This needs to be verified.

I think diverted consumption and diverted charging need handling slightly differently and therefore cannot be combined as a single diverted
I think so, too. It is required to know how much energy went to battery loading and how much went to diverting. I need to test how my battery shows the data when it switches on devices. I think in my case for "normal" electrical devices it mixes this with house consumption. It may be different if my battery switches on the heating, but I think I can not test this easily.

If a final decision was made to seperate it, I am going to create a new pull request.

@pb66
Copy link
Contributor

pb66 commented Jul 1, 2017

I certainly wouldn't make any changes until we get some more input. To be honest you would possibly get more discussion on the forum rather than here on github,

The term "use" needs to be defined, until the divert app came along use was simply derived from the sum of grid and solar, where grid is positive when importing and negative when exporting. When the divert version of the app came along "use" is presumed to be "elected use" and doesn't include diversion to DHW as that is perceived as "free energy" by many users.

Think of areas such as Spain where they are charged for exporting excess solar, in such situations users "dump" the excess to heat a volume of water purely so it doesn't get exported, that certainly isn't "use" in the expected sense of the word.

Even charging a battery isn't strictly speaking "use", it is "stored" and it gets used during discharge.

If you have a good PV day and cover your 10kWh daily consumption plus put 10kWh's away in a battery, then the next day is over cast and no PV is produced so you use the 10kWh from the battery, is your "use" 20kWh and 0kWh for those 2 days or 10kWh per day?

Likewise if you divert 3kWh's to your hotwater tank and 3kWhs to a battery, although you have diverted 6kWhs, you have neither charged your battery with 6kWh's or "used" the 6kWh's yet.

I think you would probably get more meaningful data in emoncms if you deducted the "charge" from what you currently have labelled as "use" so that you know how much you are consuming without charging the batteries ("use") separate to total self-use ("utilization"?).

@Paul-Reed
Copy link
Member

Battery storage is possibly the future, but currently not cost effective or practical, with the majority of emoncms users diverting surplus energy to heat domestic hot water instead of storing.
Needs to be a separate app.

Paul

@@ -893,14 +964,18 @@ function load_bargraph(start,end) {
if (has_wind) {
wind_kwh_data = feed.getdataDMY(config.wind_kwh.value,start,end,"daily");
}

if (has_battery) {
var discharge_kwh_data = feed.getdataDMY(config.app.discharge_kwh.value,start,end,"daily");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the same as wind_kwh_data and make it a var outside of the if and then set it inside? (I'm no JS guru, so I'm not actually sure on the difference here - scoping in JS always hugely confused me! But at least we should be consistent.)

@mattjgalloway
Copy link
Contributor

Chiming in here....

I like this! It's great to see this being built upon.

I like that it's very much additive to the one that I built. However, it's getting a bit crazy in this code now. It's annoying that there's so much shared between solar PV, solar PV + divert, etc.

@sysrqio - can you enumerate how the feeds are different with your app please? It'd be good if we can make sure that use is the same between all of the apps - that'll make it easier to refactor shared code between the apps later.

@glynhudson
Copy link
Member

Any progress on this @sysrqio? It would be really cool to get this worked out and eventually merged.

@glynhudson
Copy link
Member

Having thought more about this, I think it would be best to create a new app called MySolarBattery rather than add this into SolarPVDivert app. Since many users will want to use the divert app without a battery.

@sysrqio would you be able to copy these changes to a new app then we can get this merged

@tomascrespo
Copy link

Ohhhh what a shame, I found this PR and I get very happy, but it looks not updated.

It will be great having batteries integrated into My Solar PV App

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.

7 participants