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

Hours to Show parameter not enforced #36

Closed
arigit opened this issue Feb 6, 2019 · 12 comments
Closed

Hours to Show parameter not enforced #36

arigit opened this issue Feb 6, 2019 · 12 comments
Labels
bug Something isn't working

Comments

@arigit
Copy link

arigit commented Feb 6, 2019

Version 0.2.1

Using these parameters:

          hours_to_show: 2
          points_per_hour: 3

produces this graph:

image

where there are actually 6 hours in display, with one point shown per hour.

Testing different value combinations is showing that the "real" number of hours that are shown is actually = hours_to_show * points_per_hour.

Increasing points_per_hour to 6, produces a graph that spans for 12 hours, even if it was supposed to be showing 2 hours. It's a very strange behavior :)

In addition, using a number < 1 for "hours to show" proudces a weird result (apparently showing an entire week or so of 'hours').

There is also a problem with the labels (too much to the left and also too tiny after reducing font_size to 60) but I think there is a merged patch that would fix that in master already

@slavkobrzeg
Copy link

slavkobrzeg commented Feb 6, 2019

I also noticed very weird behaviour, but in my opinion
points_per_hour: 2 hours_to_show: 6
produces correct graph in terms of its length - 6 hours, and number of points - in this case 12, but what is wrong is actually the labels about each point. Even if the time frame it shows covers 30 minutes each and every point starts with an hour difference between other neighboring points.

tempgraph1
tempgraph2
{That 3 minutes difference is because I took second pictures 3 minutes later}

So the last point should say something about 4 AM not 11 PM previous day.
tempgraph3

So in my opinion the graph is correct, because it shows lines correctly, only the labels are not.

@kalkih
Copy link
Owner

kalkih commented Feb 6, 2019

As @slavkobrzeg and #37 says, the timestamp is definitely not calculated correctly when using the points_per_hour option.
Although I believe that the actual graph is correct, at least from my testing, as long as hours_to_show is an integer the graph will render the correct history.

Would be great if you could confirm this by maybe creating a history-graph card just below the mini-graph-card and set the same hours_to_show in the config and compare the results.

@slavkobrzeg
Copy link

I am definitely sure that the graph itself is fine. i compared it with my original temp graphs I have from the native thermostat app.

@kalkih
Copy link
Owner

kalkih commented Feb 6, 2019

There is also a problem with the labels (too much to the left and also too tiny after reducing font_size to 60) but I think there is a merged patch that would fix that in master already

The labels are getting changed in the next version, however they still scale with the set font_size, will see if I possibly can add a min size for them, where they won't scale below, thanks!

@kalkih kalkih added the bug Something isn't working label Feb 6, 2019
@kalkih
Copy link
Owner

kalkih commented Feb 6, 2019

@arigit Also added a min size to the labels font size which it won't scale below cd6d880

@arigit
Copy link
Author

arigit commented Feb 6, 2019

@kalkih : I can confirm what @slavkobrzeg mentioned - the graphs look right, just the timestamp calculations are off.
Thanks a zillion for the quick response on this!

@arigit
Copy link
Author

arigit commented Feb 6, 2019

It seems there is a fix committed to master for this already! Willing to test - but I couldn't find a test .js 'bundle' to replace the one I have installed on my raspbian-based HA install.

I have "custom-updater' installed as well and refreshing it ever now and then - it is not yet seeing the new version. @kalkih would it be possible to post a 'js bundle' somewhere that includes the latest commits so we can give it shot and confirm

@kalkih
Copy link
Owner

kalkih commented Feb 6, 2019

@arigit
It's in the dev branch and will be in the next release.
I usually don't do pre releases but sure, why not 🙂
https://github.com/kalkih/mini-graph-card/releases/tag/0.2.2-beta

Make sure to clear cache or bump the version number in the resource reference to make sure the new version loads properly.

@slavkobrzeg
Copy link

I works perfectly!

@arigit
Copy link
Author

arigit commented Feb 6, 2019

Tested - all issues fixed and it looks really great
What an outstanding addition to HA. Thanks @kalkih for this

@kalkih
Copy link
Owner

kalkih commented Feb 7, 2019

Found another issue with the reported timestamps, this time when only part of set hours_to_show was available in history, should be resolved with 2dc35b7 by rendering missing history in a straight horizontal line up to the point where history is available.

hyperion_8123_lovelace_4 21

@kalkih
Copy link
Owner

kalkih commented Feb 9, 2019

Fixed in latest v0.2.2

@kalkih kalkih closed this as completed Feb 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants