kalkih / mini-graph-card

Minimalistic graph card for Home Assistant Lovelace UI
MIT License
3.07k stars 238 forks source link

min_bound_range ignores lower_bound #1047

Closed Tarudro closed 3 months ago

Tarudro commented 11 months ago

Hi,

First of all, I love the card, thanks for this project and your effort!

However, I have noticed one thing that I can't really figure out and I believe it's kind of irritating as it is. (at least for my used case)

I have created mini-graph-cards to monitor my power consumptions. However, some devices have small values when in standby mode. I've specified lower_bound: 0 because I want to have the X axes stuck at the very bottom of the card, since I'm not expecting any negative values. Therefore a close to 0 consumption should also look like that. However, when you look at my screenshot, the "Washing Machine" doesn't look like that due to the small value scaling.

image

I checked the options and found the min_bound_range: setting.

But, when I specify this, the range neglects any defined lower_bound and creates the range to positive and negative side.

- type: custom:mini-graph-card
  icon: mdi:dishwasher
  name: Dishwasher
  hours_to_show: 6
  show:
    extrema: true
    legend: false
    average: true
    fill: fade
  line_width: 3
  lower_bound: 0
  min_bound_range: 10
  points_per_hour: 15
  entities:
    - entity: sensor.dishwasher_power
    - entity: sensor.dishwasher_energy_daily
      show_state: true
      show_graph: false
- type: custom:mini-graph-card
  icon: mdi:washing-machine
  name: Washing Machine
  hours_to_show: 6
  show:
    extrema: true
    legend: false
    average: true
    fill: fade
  line_width: 3
  lower_bound: 0
  points_per_hour: 15
  entities:
    - entity: sensor.washing_machine_power
    - entity: sensor.washing_machine_energy_daily
      show_state: true
      show_graph: false

I would suggest to make the following code change (I got no permissions to push it directly): main.js

        boundary = [
          boundary[0] - diff / 2,
          boundary[1] + diff / 2,
        ];`

And apply the offset range only to the positive side, to keep the lower_bound. image

ildar170975 commented 11 months ago

May I ask you why did you post:

?

This

However, when you look at my screenshot, the "Washing Machine" doesn't look like that due to the small value scaling.

is rather unclear.

The issue is a "bug report" for the developer. It should not be a puzzle.

As for min_bound_range question - please read Docs again about this option and compare values "10" & "5.1 + 4.9". The min_bound_range option is applied "in the end" and define a range. Since you defined the "10" value - you got this unexpected "lower_bound" negative value.

I would suggest to make the following code change (I got no permissions to push it directly):

Changes CANNOT be applied directly. Please create a PR with proposed changes.

Tarudro commented 11 months ago

Sorry, I'm not a native English speaker and couldn't express myself well enough, I guess.

I tried my best to collect all the information and I also, of course, read the documentation and read 'The min_bound_range option is applied "in the end"', but this is exactly what I tried to point out that it doesn't make sense in my point of view and use case. When a lower_bound is applied, this should still be the fixed lower_point, instead of having the range overwrite the lower_point.

But anyway, if this is not understandable or not wanted, please excuse me and don't waste your valuable time.

ildar170975 commented 11 months ago

@Tarudro Currently the card is not fully maintained. Means - not all bugs are being rectified, not all FRs are in progress. Contributors do what they can do in their available time.

Hence users have to find WORKAROUNDS in cases when something cannot be achieved explicitly. Tell us what particular goal you need to achieve with these lower_bound etc options. Probably there is a way around.

Tarudro commented 10 months ago

I totally understand!

My goal is to put the cards in a better relation to each other in regards to the Y axis of the plots and simultaneously set the lower bound to 0, such that the X-Axis basically sticks to the bottom of the card.

For example, even though my air purifier is just idling, the plot looks like it's consuming its maximum power: image

To achieve my goal, the "min_bound_range" seams feasible, because I can put multiple cards to a range for let's say 100 and the 2 Watt shown in the picture above should look like that: image

But instead, it looks like that: image

I hope I've now properly made my problem clear.

I also understand the attributes, I also understand that the range is applied as last attribute, but I thought I can't be the only one that would need a range only to the positive side of Y-Axis.

My proposed code change that I submitted in my first comment would fulfill my requirement.

The code for my cards are like this btw.

- type: custom:mini-graph-card
  icon: mdi:air-filter
  name: Dyson
  hours_to_show: 6
  show:
    extrema: true
    legend: false
    average: true
    fill: fade
  line_width: 3
  lower_bound: 0
  min_bound_range: 100
  points_per_hour: 15
  entities:
    - entity: sensor.dyson_power
    - entity: sensor.dyson_energy_daily
      show_state: true
      show_graph: false
ildar170975 commented 10 months ago

Have you tried with upper_bound specified in addition to lower_bound?

Tarudro commented 10 months ago

Thanks for your suggestion. I have, indeed. This is feasible for some devices, but not for all of them.

The benefit of having a min_range is that it still scales correctly when there are very big values coming. E.g. some appliances are at ~200W most of the times. But sometimes it goes up to 2000W. In that case I would love to keep the flexible axis instead of having it locked to 0-2000W and most of the times it's around 200W and just a tiny plot.

ildar170975 commented 10 months ago
  1. min_bound_range, lower_bound & upper_bound seem to work as documented.
  2. If there are any suggestions about these options - please propose a PR or at least a FR with a detailed description of proposed behaviour.
Joern-W commented 9 months ago

Hi, I'm struggeling with this as well. My code is:

type: custom:mini-graph-card
entities:
  - sensor.regen_heute
aggregate_func: max
hours_to_show: 120
min_bound_range: 10
lower_bound: 0
hour24: true
points_per_hour: 1
animate: true
group_by: date
icon: mdi:water
bar_spacing: 20
line_color: '#1585e5'
height: 180
show:
  graph: bar
  state: true
  points: true
  labels: true

Note the two lines min_bound_range: 10 and lower_bound: 0

This is the result: grafik

The expected behaviour would be: The lower bound is always 0, the upper bound is 10 or higher, depending on the data.

I hope, this clarifies the problem.

Regards Jörn

ildar170975 commented 9 months ago

Note the two lines min_bound_range: 10 and lower_bound: 0

As it was clearly said in Docs & explained already in this thread - the min_bound_range is applied at the end. Suggest to not use min_bound_range in your case - the lower_bound could be enough. Alternatively, use min_bound_range only if you are interested in the fixed minimal range.

This issue will be closed soon. Described scenarios do not contradict Docs, no alternative solutions were provided in PR (I could agree that the current implementation MAY not cover all users' needs - but there are no PRs from people who need a different behaviour).

Tarudro commented 9 months ago

Hi,

I have proposed an alternative solution, even proposed the source code change. From my view the documentation covers it, but at the same time it is also unexpected and only specifying the lower bound is also not bringing the desired solution of a fixed y axis scaling.

It could be a very easy change and also updating the docs accordingly would make sense.

However, after pointing it out I really didn’t feel like the feedback is appreciated, so I also gave up.

I don’t have the time to setup a development environment to verify any changes I propose, unfortunately.

If our problems and proposals don’t match your paradigm, just close it, sure. No hard feelings. Came here with a good intention. And also happy to realize that I’m not the only one.

cheers

ildar170975 commented 9 months ago

If our problems and proposals don’t match your paradigm, just close it, sure. No hard feelings. Came here with a good intention. And also happy to realize that I’m not the only one.

This is not about "don’t match your paradigm". We have very limited possibilities to maintain the project - I have already explained this to you, sad that have to come back to this again. Proposals are better to be "proposed" as ready PRs. Ofc some proposals may be implemented using just a detailed description - but this depends on a particular developer if he/she decides to spend his/her time for this. Sad but true.

Well, I may leave this issue open - but it may not change things.

ncd7 commented 4 months ago

Try specifying it this way using the tilda sign. It acts as a soft bound but can be 'breached'. E.g.

lower_bound: 0
upper_bound: ~50

This way by default the bounds will be 0 - 50 and Y-axis will be scaled that way. However, if values above 50 appear, they will still correctly push up the Y-axis to go from 0 to that new higher-than-50 bound.

I didn't read the entire thread but the first few messages and I think this should help you achieve what you want.

Joern-W commented 4 months ago

Great! Thank you @ncd7! This works!

ncd7 commented 4 months ago

Great, glad it helped. In this case I'd recommend closing this issue since the existing features are actually rich enough to satisfy your requirements, no dev work needed.

ildar170975 commented 4 months ago

I already proposed to close the issue several times - and suggested to create either a PR or a FR with a DETAILED algorithm of needed changes (so some developer takes this algorithm & writes a code).

Tarudro commented 3 months ago

@ncd7 thank you for that hint! it really satisfies my requirement.

It's not intuitive to find, also when checking the documentation. But probably others will also find this issue :-) Thanks for your support

FAB1150 commented 2 months ago

@ildar170975 Sorry that I'm replying to an already closed issue, but it's not too clear from the docs so I came here searching if somebody had encountered my problem and I noticed that you said

Proposals are better to be "proposed" as ready PRs.

The "CONTRIBUTING" guidelines directly contraddict this, as the "Important notes for pull requests" says

As the project maintainers are currently very limited in time, please discuss new features and enhancements via an issue first

Now, I would actually agree with you for a small project mantained by a few people in their spare time, but it's a bit unprofessional (whatever that means) to get that aggressive towards a user that is doing exactly what the guidelines of the project say to do. Either change the guidelines (I can do a PR of that if you wish), or follow them.

Politeness aside, I think it would be a good change to the behavior of min_bound_range: WHEN min_bound_range > range, IF no bounds set: usual behavior IF upper bound set: lower value get smaller IF lower bound set: upper value gets bigger IF upper AND lower bound set: usual behavior

I'm not too good at js, but I can try to do a PR if I find the time. Sorry for the rant, no need to answer if you don't want to. Have a nice day

ildar170975 commented 2 months ago

@FAB1150

directly contraddict this

There is no contradiction. Some discussion took place here and it came to a stage when a concrete detailed description was needed. Since it was not provided in a ready "descriptive" form - I suggested to issue a PR with a particular code.

but it's a bit unprofessional (whatever that means) to get that aggressive

Please do not tell me what an "aggression" is, I see it almost every day by my eyes. You are rather exaggerating. Here it was just a polite request from my side about a need of a detailed description.

but I can try to do a PR

This will be very productive! Unfortunately, the card's code is obfuscated - so you cannot add your changed lines locally into a js-file & test; instead, you will have to compile a js-file & test then.

FAB1150 commented 2 months ago

@ildar170975 If you wanna check it's PR #1136 :)

ildar170975 commented 2 months ago

@FAB1150 Very good ! Will take a look in a week when get to PC.