jerbzz / pi-eco-indicator

Display at-a-glance data of carbon intensity or Octopus Agile prices on a Pimoroni Blinkt! display or a Pimoroni Inky pHAT display.
BSD 3-Clause "New" or "Revised" License
26 stars 5 forks source link

Updated the Chart height #14

Closed Joeboyc2 closed 3 years ago

Joeboyc2 commented 3 years ago

On the new inky displays, the values used for the charts y height causes it to overlay the current value text when using agile_price mode Adjusting this down makes the display more legible

Before After
20210929_092210 20210929_093102
jerbzz commented 3 years ago

I'm going to (temporarily) fix this differently by changing the overall y-height for both displays. This is happening because of the current bonkers Agile prices - previously, it was rare to hit the 35p cap at all so there would only be a little bit of occlusion. It would be nice to draw a black outline round the text too... hmmm.

jerbzz commented 3 years ago

See also https://github.com/jerbzz/pi-eco-indicator/issues/9

Joeboyc2 commented 3 years ago

Hi @jerbzz , I also thought about having a black outline around the text as that I feel would at least help the text to stand out. But I really didn't know where to start :P I'll take another look at the code and see if I can work it out, I'll submit another pull request if I find a way before you do :)

jerbzz commented 3 years ago

Last time I thought about it, I seem to recall finding that Pillow (the image drawing library we are using) does not support text outlines natively. So we would probably need to draw the numbers in black four times, shifted up/down/left/right one pixel each time, and then draw them in red on top. But only if we are already going to draw them in red.

jerbzz commented 3 years ago

https://github.com/jerbzz/pi-eco-indicator/commit/67dd36ce6b04d3c29f28dd18f2c30f388e7ecc1a

Here ya go, if you feel like updating your repo (not to 2.1.0, it's in main but not tagged a release yet) and giving it a test, that'd be super :)

Joeboyc2 commented 3 years ago

Hey @jerbzz , I can see you have been busy and have updated the file a few times recently. When I try to update the screen using the version of the file you have tagged above or the later version of the file with the comment "special case for when the lowest slot is now." I get this in the console:

pi@AgileRates:~/pi-eco-indicator $ ./update_display.py Connected to database... Traceback (most recent call last): File "./update_display.py", line 48, in config = eco_indicator.get_config() TypeError: get_config() missing 1 required positional argument: 'filename'

I have taken a look through the updates you have been making and I can see all the references to 'config.yaml" has been replaced with the term 'filename', I assume this is supposed to be a variable but I can't see that it is being set anywhere.

I can also see that changes for 2.1.0 show that the config should be able to be specified using either -c or --config but having tried that I also fail :(


having written all the above, I have just seen that 2.1.0 has actually been released :P I will update to that to see how I get on 👍🏼

Joeboyc2 commented 3 years ago

Running the new code and all looks good, no issues with running the commands, thank you for all the updates

Joe

jerbzz commented 3 years ago

Hey,

So the latest code with auto scaling etc isn't tagged as a release at all, yet, because it needs testing.

Sounds like for your error you had some of the changes but not others.

The easiest way to make sure that you have the latest code (development version) is to delete your pi-eco-indicator directory, then

cd ~

git clone https://github.com/jerbzz/pi-eco-indicator

Then follow normal setup steps of copying config.yaml.default to config.yaml

etc :)

Joeboyc2 commented 3 years ago

Hey @jerbzz, I actually re-downloaded all the code again, using the code from the readme

cd ~ && git -c advice.detachedHead=false clone --depth 1 -b v2.1.0 https://github.com/jerbzz/pi-eco-indicator.git

having install that and re-setup the config it chart actually looks good, so I guess some of that code made it in..... 20211011_080136 Im happy with the way it looks now, based on running that code pull above,

jerbzz commented 3 years ago

2.1.0 includes the bodge that you and I made to get the graph from behind the numbers when the prices are bonkers, but not the auto scaling. :)

Joeboyc2 commented 3 years ago

ah ok, Let me give the change above a go no I have update the base code, I'll come back to you in a min

Joeboyc2 commented 3 years ago

Ok, so I have updated the code to what was in the change above, and it doesn't look as good 20211011_081748

so I have updated the value in line 157 from:

graph_y_unit = (inky_display.HEIGHT / 2.5) / max_slot_value

To

graph_y_unit = (inky_display.HEIGHT / 2.8) / max_slot_value

and that looks a little better: 20211011_083317

jerbzz commented 3 years ago

And this is why testing is great!

I set that using my older display and it was great at 2.5.

So what we need to to do fix that is make that value a variable, and then set it further up where the display specific scaling values are set. Call it y_scale_factor or something. Clearly does need to be 2.8 for the newer displays but 2.5 was right for the older displays.

Make sense?

Joeboyc2 commented 3 years ago

ok, so I have made an amendment to the file.

I'll raise another pull request and we can discuss it over there :)