Fix state class of bpRemainWatth to allow decreasing values#60
Fix state class of bpRemainWatth to allow decreasing values#60MarijnS95 wants to merge 2 commits intoniltrip:mainfrom
bpRemainWatth to allow decreasing values#60Conversation
| "watth": ( | ||
| SensorDeviceClass.ENERGY_STORAGE, | ||
| UnitOfEnergy.WATT_HOUR, | ||
| SensorStateClass.MEASUREMENT, | ||
| ), |
There was a problem hiding this comment.
Drive-by change: this now not only uses MEASUREMENT over TOTAL_INCREASING, the device class was updated from ENERGY to ENERGY_STORAGE, better covering the fact that it's a battery (and hass automatically assigns it a more fitting battery icon.
There was a problem hiding this comment.
That looks very good. I just wanted to read data from my system and hoped someone here would continue this project. My system has been running for over two years now. I'd like to take over this, but I'm currently doing a huge overhaul in my other branch. Since I'm not very familiar with merging, rebasing, etc., it would be great if you could take a look at the other branch. I'm open to suggestions. At the moment, I'm trying to implement the concept with devices. Every serial number I find in the JSON should become its own device with parameters. I've also deleted a lot of things or had AI optimized. Thanks.
By the way, do you own a PowerOcean system yourself, or are you just interested in coding?
My hope was that the "official" ecoflow_cloud integration would include PowerOcean. Now it seems to be slowly gaining momentum, but I'm still getting more data points this way.
There was a problem hiding this comment.
Hey, sorry for the late response.
Yes we just got a PowerOcean Pro two weeks ago and I was evaluating and testing the integrations. Yours being the only one that seems to be applicable for now.
I've been meaning to ask, what are your plans to merge this back to "the official upstream" ecoflow_cloud integration? It's very hard (not to mention annoying) for new users to find all these many forks and variations of integrations (but at the same time it's a thankless job to test and and maintain and provide everything, which I definitely appreciate).
It seems that "upstream" MQTT public_api-based support was merged for your non-Pro PowerOcean, right? Are there any advantages from your end to stick to the JSON API ("more data points" as in more frequent updates)? How about a plan to contribute JSON-based support as a third option to the "official" integration?
I've been working on reverse engineering the MQTT private_api protobuf packets from the Pro, but the upstream repository is an incredible mess to the point where I almost regret using it as a base, but it's the most popular already. It's so heavily intertwined with HASS even deep down in per-device support that it's almost untestable outside. But again: I really appreciate everyone working on and contributing to these repositories. It's quite easy to get an integration working and share it with the world, but then maintaining and extending it while also targeting HASS's constantly moving "best practices" requires nontrivial commitment.
I like your idea of splitting out the devices (using a single hub) but haven't taken a close look yet. Summary from the above: based on the plans for the "official" integration, does it make much sense to spend a lot of time here?
It's okay for me to wait on your progress and rebase this on top, it's a relatively small change after all.
I can't recommend using AI for this, especially not "optimization". All my experience with it is that it generates an incomprehensive and extremely overcomplicated codebase that becomes worse with every addition or refactor. "Keep It Simple, Stupid" has my preference but that sometimes requires wiping everything clean and starting over 😅
There was a problem hiding this comment.
I think I have done this in #58 .
Im not very familiar with merging from to and rebase .....
I prefer to push my branch to main @MarijnS95 @jdammers .
feature + bug fixing
every SN is a device
coordinater for value update
rework config_flow
fix bpmainWatth with the great idea with _attr @MarijnS95
Would be great if someone can check.
I run it parallel to my powerocean in container.
@MarijnS95
Long story short: When I first set up my PowerOcean system, there was no working integration. Using a private API and protobuf was too complicated for me. A good approach worked in ioBroker with foxthefox. He handled the protobuf part but was a dedicated ioBroker user (proxmox > iobroker > mqtt > HA ! keep it simple !). When I experimented with ecoflow_cloud and the public API (pull_request), I found it didn't provide enough data. Now my integration is just a private project ;-). After two years, I have to figure out how I would ever migrate to another integration...
My integration uses the Webinterface (no mqtt). For some values it is helpful to have the app open.
The important data is coming in.
There was a problem hiding this comment.
the ecoflow_cloud 1.4.0 beta 9 is a good step in the right direction.
@MarijnS95 do you have an idea to get an icon for my integration? I tried but failed. 😅
c24f896 to
79702f6
Compare
Fixes #51
Closes #57
The sensor state class of all entities with the
Whunit are marked asTOTAL_INCREASING, while thewatthkey suffix is used for parameters that describe the current charge of the battery which can go up or down and requires theMEASUREMENTstate class instead.Instead of trying to handle this at the unit level, change the existing mapping from a key suffix to a unit to also immediately determine the sensor device and state class as these are all closely related to each other.
With that it seemed more clear to handle them all in the same place rather than coming up with three independent mappings based on keys, but the resulting tuple passing proves less than ideal (especially with the already-verbose
PowerOceanEndpointdata passing). On that note I do wonder if we could just assign the_attrfields rather than overriding the properties: https://developers.home-assistant.io/docs/core/entity/#entity-class-or-instance-attributes.