bimmer_connected icon indicating copy to clipboard operation
bimmer_connected copied to clipboard

Add charging sessions and charging statistics data for hybrids and BEVs

Open cubinet-code opened this issue 2 years ago • 5 comments

Proposed change

This enhancement adds charging statistics and charging session information provided by the connected drive API for hybrid and BEVs.

  • Collects the current months data only, as to reduce the risk to run into rate limiting of the API servers
  • To keep code consistent, it has been implemented matching the logic that was used for "charging_settings".
  • Additional tests and test live-data from a U11 have been added to support the change
    • test_charging_type_unknown
    • test_charging_sessions and charging blocks
    • test_charging_statistics
    • test_charging_type_unknown
  • Prepared to expose software_version, total_energy_charged, charging_session_count to homeassistant component

Type of change

  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] New feature (which adds functionality to this library)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

A preliminary review would be appreciated to understand if my PR meets the project requirements. Unfortunately I do not have an ICE vehicle available in my account. While the tests with the existing vehicle data passes just fine, it would be very interesting to run the fingerprint against an ICE vehicle to understand the API results if no charging information is available.

  • This PR is partly addressing feature request #402

Checklist

  • [x] The code change is tested and works locally.
  • [x] Tests have been added to verify that the new code works.

cubinet-code avatar Sep 20 '23 10:09 cubinet-code

Thanks! Need to have a look in more detail later - maybe I get the chance on the weekend.

Two things came to my mind when looking at the changed files:

  • Did you create the JSON files through the fingerprint function? If so, we should make sure that e.g. address is anonymized correctly.
  • Also our internal request queue for debugging should be increased, if we get alone 8 requests for the statistics

rikroe avatar Sep 21 '23 19:09 rikroe

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (761b4a8) to head (7d1d127). Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #569    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           17        19     +2     
  Lines         1355      1512   +157     
==========================================
+ Hits          1355      1512   +157     
Flag Coverage Δ
3.10 100.00% <100.00%> (ø)
3.11 100.00% <100.00%> (ø)
3.12 100.00% <100.00%> (ø)
3.8 100.00% <100.00%> (ø)
3.9 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 21 '23 20:09 codecov[bot]

Thanks for the hints!

I updated the anonymisation dict to remove additonal charging related private data.

Also, increased response queue depth to 45 with the following rationale:

  • 4 * base vehicle data
  • 1 * charging statistic
  • 40 * possible monthly charging sessions, should not be more than 1 per day)

cubinet-code avatar Sep 22 '23 07:09 cubinet-code

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Dec 22 '23 03:12 github-actions[bot]

Hi, I'm sorry to leave you hanging for that long. Other priorities...

I thought about your changes and think that we should not request all those endpoints everytime a normal refresh is done (i.e. every 5 minutes as default for Home Assistant). We've had rate limiting issues in the past and I dont want to increase the load to BMW servers too much if possible.

How do you feel about a solution similar to a remote service? I.e. you have to manually request the historic data, like triggering the vehiclefinder service.

The result could be either stored in a full blown object (like you have already implemented) or even just returned as plain JSON - depending on how you want to continue working with the data.

rikroe avatar Jan 21 '24 13:01 rikroe

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Apr 21 '24 03:04 github-actions[bot]

Will this feature be reviewed and added in short term ?

fideias avatar Jun 03 '24 15:06 fideias