Skip to content

fix issue with incorrect state class (bpRemainWatth issue)#57

Closed
jdammers wants to merge 7 commits intomainfrom
fix_bpremainwatth
Closed

fix issue with incorrect state class (bpRemainWatth issue)#57
jdammers wants to merge 7 commits intomainfrom
fix_bpremainwatth

Conversation

@jdammers
Copy link
Copy Markdown
Collaborator

I have set the state class to SensorStateClass.MEASUREMENT and modified SensorMapping.get_sensor_state_class in sensor.py
After these changes I don't see any logs referring to the wrong state class for sensor.powerocean_bpremainwatth

@niltrip
Copy link
Copy Markdown
Owner

niltrip commented Jan 18, 2026

Hi, nice to hear from you. I had already made a fix, but I generally changed Wh in STATE_CLASS_MAPPING to
"Wh": SensorStateClass.MEASUREMENT. At least that's how I read it in the documentation.

But I've now seen that Wh is also used in "bpTotalChgEnergy" and "bpTotalDsgEnergy". These values ​​just keep increasing.

Your solution might be better after all.

I'll have to take another look.

@MarijnS95
Copy link
Copy Markdown

Thanks @jdammers! Don't forget to edit your pull-request description to include "Fixes #51" so that the original issue report is linked and will be auto-closed when this PR is merged. 🙏

@MarijnS95
Copy link
Copy Markdown

MarijnS95 commented Jan 28, 2026

First and foremost: can you update the title to be more descriptive? bug fix says very little, something like Fix state class of `bpRemainWatth` to allow decreasing values much better covers the load.


Looking at this change a bit more, would it perhaps be clearer if we move this SensorMapping.get_sensor_state_class() based on the unit alone into SensorMetaHelper and base it on the key name instead?

It already tells the two cases apart by the Energy vs Watth suffix, the former is strictly increasing while the latter is not.

@jdammers jdammers changed the title bug fix fix issue with incorrect state class (bpRemainWatth issue) Jan 29, 2026
@jdammers
Copy link
Copy Markdown
Collaborator Author

@niltrip I revised the changes to address the comment from @MarijnS95

Fixes #51
This PR addresses the issue where bpRemainWatth sensor had incorrect state class that didn't allow decreasing values.
The following changes were made:

  • Move state class determination logic from SensorMapping to SensorMetaHelper
  • Distinguish between Energy (total_increasing) vs Watth (measurement) suffixes
  • Allow bpRemainWatth to use measurement state class for decreasing values

@niltrip
Copy link
Copy Markdown
Owner

niltrip commented Feb 1, 2026

Hi everyone, I have another idea I'd like to discuss.

change
Wh in STATE_CLASS_MAPPING to
"Wh": SensorStateClass.MEASUREMENT in sensor.py.

change

Screenshot 2026-02-01 115529

I don't even remember why we treated "Generation" as a special case here.

change

Screenshot 2026-02-01 115550

The displayed values ​​for bpTotalChgEnergy and bpTotalDsgEnergy change to kWh.

"kWh": SensorStateClass.TOTAL_INCREASING

I can do more with kWh.

grafik

@MarijnS95
Copy link
Copy Markdown

@jdammers unfortunately that isn't entirely what I meant. Would you mind if I open a PR with my version of these changes, that would save some of the back-and-forth?


@niltrip

I don't even remember why we treated "Generation" as a special case here.

Looking at the history, it seems that only a few specific keys were mapped to units, and everything which contained Generation. Later changes switched to suffix-matching, without also including energyGeneration which is also always a suffix as far as I can tell from the JSON dumps.

It seems fine to leave *Energy in Wh; that's the native format and stays in integer (no floating-point decimals).

(Also it's really hard to work off of screenshots, having a git diff or branch is much more natural to look at 😉).

@niltrip
Copy link
Copy Markdown
Owner

niltrip commented Feb 1, 2026

@MarijnS95
Hi, this is a screenshot of git diff ;-)
I'm not a programmer.

Feel free to create a pull request.
I'm currently working on a different topic (#58).
Yes, the value is in Wh, but kWh makes more sense here, since "bpTotal*" only increases.

You're absolutely right about the integer, but i did not change the value!

@jdammers
Copy link
Copy Markdown
Collaborator Author

jdammers commented Feb 1, 2026

@MarijnS95 @niltrip
Fine with me - so yes please go ahead! Thanks!

@MarijnS95
Copy link
Copy Markdown

@niltrip you still got a lot of things done for "not being a programmer", kudos! Forgot to mention here to @jdammers that that PR is open at #60. I also left a bit of an offtopic comment on the situation with there being many concurrent EcoFlow integrations out in the wild already, and plans to condense...


You're absolutely right about the integer, but i did not change the value!

That's the / 1000 which converts it from int to float.

@niltrip
Copy link
Copy Markdown
Owner

niltrip commented Feb 14, 2026

Thanks for the work i integrate the fix on my branch

@niltrip niltrip closed this Feb 14, 2026
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.

3 participants