Skip to content

Add example using line for a moving sparkline graph #14

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

Merged
merged 25 commits into from
Jul 22, 2020

Conversation

kmatch98
Copy link

@kmatch98 kmatch98 commented Jun 7, 2020

Inspired by this Arduino library for sparklines, here is an example of creating moving sparkline graphs in CircuitPython using the Line command.
F86221FE-3CF5-4CD2-9103-A8EFDA74616D

This example includes the sparkline class definition and a practical example that creates three different sparklines with bitmap backgrounds.

Class usage: sparkline

A sparkline is instanced with the following input variables:

  • width: Width of the sparkline in display pixels
  • height: Height of the sparkline in display pixels
  • max_items: Maximum number of values that will be stored and displayed in the sparkline
  • yMin: The lower y-range of the bottom of the graph. Set to None to autorange the bottom axis (Default=None)
  • yMax: The upper y-range of top of the graph. Set to None to autorange the top axis (Default=None)
  • line_color: The color value used for displaying the line (Default=0xFFFFFF white)
  • x: x-position of the upper left corner of the sparkline display, in pixels (Default=0)
  • y: y-position of the upper left corner of the sparkline display, in pixels (Default=0)

This sparkline class includes three class functions:

  • add_value: Add a value to an existing sparkline
  • update: Updates the visual elements of the sparkling
  • values: Returns the current list of values stored in sparkline

@kmatch98
Copy link
Author

kmatch98 commented Jun 7, 2020

The checkout failed pylint, but the errors where on files that were not modified in this pull request. I think it didn’t get to my new example before it gave up. I haven’t got pylint running on my local machine yet, but will fight through getting it running if you need me to (kattni’s pylint tutorial looks like a godsend based on my first glance at it).

@ladyada ladyada requested a review from makermelissa June 7, 2020 16:17
@FoamyGuy
Copy link

I looked into these PyLint issues a bit and I'm a perplexed by them. When I try to run PyLint locally I am not getting the same failures:

Adafruit_CircuitPython_Display_Shapes_kmatch98>pylint --rcfile .pylintrc adafruit_display_shapes\circle.py

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

I'm not sure why I'm getting differing results from the github CI system.

Focusing on the issues that it shows in the github check:

I think renaming x0 and y0 variables to x_0 and y_0 would fix the invalid-name issues.

The not enough public methods issue I don't know how to solve other than disabling it. Circle and Line are sub-classes of other things that include public methods I think. They don't change the behavior of any public methods or add any new ones. They only have a modified __init__()

@FoamyGuy
Copy link

Pylint oddities aside I think this is a neat addition but can be refactored a bit.

I would propose that the sparkline class from this example get moved into the library in it's own file adafruit_display_shapes\sparkline.py and within the file the class name should be changed to uppercase class Sparkline

The example script can remain as is (minus the class) to show an example of usage. I can help update the example to work with built-in displays as well.

@timonsku
Copy link

This is great! +1 to the refactor and making the class part of the library.
I tried it out on a Feather M4 with the Adafruit 2.0" 320x240 ST7789 display and works well except for some form of ghosting?
Here is a video of it: https://imgur.com/m6NDLV7

@kmatch98
Copy link
Author

I separated out the class into a separate Sparkline class. Also, I am including three examples:

  1. Basic sparkline
  2. Sparkline graph with y-axis tick marks and two labels
  3. Three sparklines with some text, including one graph with autoranging and auto update of the y-min and y-max label

@FoamyGuy I will appreciate if you will take a look to verify the examples.
Also, I use my specific display driver for my examples, is there a better option for a generic form for setting the display?

@FoamyGuy
Copy link

FoamyGuy commented Jul 21, 2020

This is great! Thank you for refactoring it into a class similar to the other shapes in the library. I tested all 3 examples on PyPortal and they are are all working for me on that device.

I do see a bit of the "ghosting" effect that PTS93 mentioned. I think perhaps it's the previous frame in the process of being deleted maybe? I'm going to try to poke around a bit inside the Sparkline class tomorrow to see if I can understand it a bit better.

I do think it will be best to at least provide the option to use built-in display in the sample code. I can submit some PRs on your branch to add this. Maybe we can structure it to try the built-in display first and if it doesn't find one then use the display driver to set it up. That way it can avoid the importing of the driver on devices that don't need it.

Unrelated to the specific PR, but once this gets merged in I think it would be great to built some accelerometer examples with it.

Also it seems like maybe the Actions system is having some trouble at the time of writing this comment. Hopefully it will eventually run those checks. But worst case scenario I think it should try again with the next commit.

@kmatch98
Copy link
Author

Thanks for the comments on the ghosting. I alleviated the worst of it by turning the display auto refresh off during the update. However I am somewhat torn about having a separate add_value and update functions. I actually went back and forth with this. I originally put these two separate because I was trying to minimize the amount of code to run just associated with updating the display. The concept was that might help speed up the updating of the screen.

I actually think it’s simpler to use if the add_value automatically triggers an update. What do you think?

I think the ghosting is just something with how displayio does the redraw. I don’t think it clears the display fully before redrawing. If you plot one full screen bitmap and replace it with another, you can see how displayio is redrawing. I don’t suspect this can be solved inside Sparkline. But I definitely agree that it’s not desirable and I’m all ears for a solution, but I suspect it will need to be done in displayio.

As for the display setup to be more generic I will appreciate your input on that.

@kmatch98
Copy link
Author

Wow, got this to survive pylint! Anyway, I am satisified with the examples now. Please review and let me know what updates are required to merge. Thanks for all your help!

Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Hi, as per PEP8, could you please use snake_case variables instead of camelCase? You shouldn't need to pylint disable "invalid-name" then. This is so we can keep things consistent. Thanks.

@makermelissa
Copy link
Collaborator

Actually, you might still need the invalid-name thing for x and y, which is fine.

@kmatch98
Copy link
Author

Ok will do. I have to learn the “way of pylint” to save effort next time. Will update and enable that check and let you know.

@kmatch98
Copy link
Author

@makermelissa per your request, I resolved and and re-enabled "invalid-name" pylint checks for sparkline.py and it currently passes with some remaining "disables". The examples are currently passing without any "disables".

The following pylint disable= settings remain. I can deal with line 101, but the others are not clear to me. It will take some time for me to resolve them. Suggestions are welcome.

Line 42: too-many-arguments
Line 101: no-else-return
Line 123: too-many-branches, too-many-nested-blocks

@makermelissa
Copy link
Collaborator

Thanks @kmatch98. The other disables are fine. I'll test these examples out this afternoon.

Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

I tested on the PyPortal. The first 2 examples worked fine, but on the "triple" one, I get 'DISPLAY_WIDTH' is not defined and that's probably because DISPLAY is part of board, so that definition is bypassed.

@makermelissa
Copy link
Collaborator

Perhaps you can set DISPLAY_WIDTH to board.DISPLAY.width so that it works on the Titano as well.

@kmatch98
Copy link
Author

@makermelissa Unfortunately I don't have access to pyPortal or Titano to evaluate this fix. I added your suggested code. If you are willing, please check if this resolves the issue.

If there are more complicated changes required, I'll have to rely on @FoamyGuy to verify if this works on the different flavors of pyPortals, he developed this cool way of checking for internal vs external display.

Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Tested good. Thanks!

@makermelissa makermelissa merged commit 5dee81e into adafruit:master Jul 22, 2020
@kmatch98
Copy link
Author

Excellent. Thank you!

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants