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

Timeline: fix kWh values and column layout for miles (#3846) #3847

Closed
wants to merge 1 commit into from

Conversation

sdwalker
Copy link
Contributor

Add the kWh multiplication needed for driving and parking when using miles
Use the same index value for the Range Diff column so the layout is the same with miles or kilometers

Add the kWh multiplication needed for driving and parking when using miles
Use the same index value for the Range Diff column so the layout is the same with miles or kilometers
Copy link

netlify bot commented Apr 16, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 20a2c5a
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/661deb03a2bf83000851a643
😎 Deploy Preview https://deploy-preview-3847--teslamate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cwanja cwanja linked an issue Apr 16, 2024 that may be closed by this pull request
1 task
@DrMichael
Copy link
Collaborator

May I propose to use convert_km instead of CASE in the SQL statement? Just to be consistent.

@JakobLichterfeld JakobLichterfeld added the area:dashboard Related to a Grafana dashboard label Apr 17, 2024
@sdwalker
Copy link
Contributor Author

sdwalker commented Apr 18, 2024

May I propose to use convert_km instead of CASE in the SQL statement? Just to be consistent.

convert_km divides by 1.60934 so the proper fix for charging looks to be to not use convert_km at all

-convert_km((end_[[preferred_range]]_range_km - start_[[preferred_range]]_range_km)::NUMERIC, '$length_unit') * efficiency AS "kWh",

+(end_[[preferred_range]]_range_km - start_[[preferred_range]]_range_km)::NUMERIC * efficiency AS "kWh",

Still looking at how to fix the parking query

convert_km(((LEAD(d.start_[[preferred_range]]_range_km) over w + (LEAD(d.start_km) over w - d.end_km)) - d.end_[[preferred_range]]_range_km)::NUMERIC, '$length_unit') * efficiency AS "kWh",

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label May 19, 2024
@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Jun 11, 2024

Still looking at how to fix the parking query

Are you planning a separate PR or include in this one?

Also please update your branch with latest master

@github-actions github-actions bot removed the Stale label Jun 12, 2024
@sdwalker
Copy link
Contributor Author

Can we assume the parking query is correct for metric users?
If so, a metric user would be able to remove the convert_km from the parking query, switch their units and confirm the unit conversion matches

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Jul 23, 2024
@dyxyl
Copy link
Contributor

dyxyl commented Sep 6, 2024

Can we assume the parking query is correct for metric users? If so, a metric user would be able to remove the convert_km from the parking query, switch their units and confirm the unit conversion matches

Yes, both the driving and parking kWh are the same in km units if the convert_km is removed so that looks like a good solution to me.

@github-actions github-actions bot removed the Stale label Sep 7, 2024
Copy link

github-actions bot commented Oct 7, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Oct 7, 2024
@github-actions github-actions bot removed the Stale label Oct 20, 2024
@JakobLichterfeld JakobLichterfeld marked this pull request as draft October 21, 2024 12:09
Copy link
Contributor

@swiffer swiffer left a comment

Choose a reason for hiding this comment

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

efficiency is stored as kwh/km, therefore convert_km seems not to be needed at all in these calculations, agree.

@dyxyl - do you want to update your PR based on latest master or should I open a follow up PR ?

@dyxyl
Copy link
Contributor

dyxyl commented Oct 28, 2024

I don't have a PR for this -- it was @sdwalker who originated it.

@swiffer
Copy link
Contributor

swiffer commented Oct 28, 2024

😆 - sorry. @sdwalker, would you like to update the PR ?

@swiffer
Copy link
Contributor

swiffer commented Nov 19, 2024

with #4367 merged this one can be closed @JakobLichterfeld

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dashboard Related to a Grafana dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeline kWh is incorrect for miles
5 participants