Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

Improved use of memory in ScreenPlotter, some bug fixes and an enhancement #24

Merged
merged 13 commits into from
Feb 9, 2022

Conversation

kevinjwalters
Copy link

@kevinjwalters kevinjwalters commented Jan 6, 2021

The plotters_combined example dies with a MemoryError on a Feather nRF52840 Express with a PMS5003 attached to an Enviro+ FeatherWing. These changes address this.

Tested with plotters_combined.py and plotter_test.py on Feather nRF52840 Express with 6.0.0.

  • plotters_combined runs fine with PMS5003 in passive mode for at least 12.5hrs at interval = 0.2 (code floors at 1s) based on testing. It starts around 53k free memory and stays constant at 30k once it starts scrolling.
  • plotters_combined runs fine with PMS5003 in active mode for at least 5hrs, rest is same as above.

No dependency on #21 but that could be reviewed at same time as this and applied beforehand.

Kevin J Walters added 11 commits January 3, 2021 15:42
Removing unnecessary per-draw display.show() invocation.
In draw() turing off display.auto_refresh if on at start and restoring it at end.
The pixels for each column are now undrawn first for all lines and then drawn back unconditionally to deal with pixel overlaps.
Default is now based on the pixel dimensions of the screen but can be specified with width, height kwargs.
…ffer pimoroni#19.

A single list is now preallocated at a fixed size and not replaced with slices to reduce continual allocations and fragmentation.
Includes fix for calling update() several times before draw().
The full_refresh implementation now only clears the bitmap when it scrolls.
Removed the commented-out code.
pimoroni#19.

Adding new test with some patterns and variable number of values to look for bugs around scrolling.
Re-implemented with a keyword argument (auto_show) to control whether ScreenPlotter invokes show().
plotters_combined depends on this functionality.
Adapted tests in plotter_test.py to use both approaches.
…exceeded. pimoroni#19

Discarding data which cannot be stored by update() if internal buffer capacity is exceeded, exceptions can be thrown if auto_discard is set to False.
plotters_combined relies upon data being discarded by update(whatever, draw=False) for plots not on screen.
Updating plotters_test to perform 32 undrawn update() in test_twolinesfewdraws.
@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 7, 2021

@dglaude This looks good if you wish to test on your FeatherS2 setup with Enviro+ FeatherWing and PMS5003. This goes with the standard Pimoroni setup described in https://www.instructables.com/Using-the-Pimoroni-Enviro-FeatherWing-With-the-Ada/ and pimoroni/pms5003-circuitpython#8

@kevinjwalters kevinjwalters changed the title NOT YET FINISHED - Improved use of memory in ScreenPlotter, some bug fixes and an enhancement Improved use of memory in ScreenPlotter, some bug fixes and an enhancement Jan 7, 2021
@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 7, 2021

@Gadgetoid This together with pimoroni/pms5003-circuitpython#8 makes the Enviro+ FeatherWing more robust and more memory efficient, let me know what you think.

If/when this is all applied it would be great to get a refresh of submodules and a new release for this.

@kevinjwalters
Copy link
Author

kevinjwalters commented Apr 17, 2021

I've half broken the page turning for plotters_combined when interval isn't 1 second. I'll fix this up in a day or two. FIXED with 0c2cf96

@kevinjwalters
Copy link
Author

I've effectively tested this on 6.2.0 too while developing support for SCD-30 CO2 sensor.

@kevinjwalters
Copy link
Author

@Gadgetoid If you've got the FeatherWing out you might want to review the four outstanding (in every respect) PRs for this repo.

@Gadgetoid
Copy link
Member

Of course I attempted to merge the other, smaller PRs and made a total hash of things- conflicting this PR.

Sorry for the lack of response here, I've been totally overwhelmed with a mixture of life and the million other things vying for my attention.

I think I've got an EnviroPlus FeatherWing that survived my move down-south somewhere, but your incredible level of diligence here makes me extremely tempted just to merge 😄 (I don't have more than one Feather board to test with at the moment 😬)

@kevinjwalters
Copy link
Author

I did a lot of testing but it's always possible to miss something. That was all pre CircuitPython 7 and only on Feather nRF52840. Worth giving it all a go on an M4 board with CPy 7.

@Gadgetoid
Copy link
Member

Gadgetoid commented Feb 9, 2022

Okay, merge conflict fixed!

I say we do or die with this 🚀

Oh and- thank you, very much! This project is a little bit on the fringe of my knowledge so it's difficult to keep things tip top, though I'm slowly starting to work more generally with CircuitPython.

@Gadgetoid Gadgetoid merged commit 6011f58 into pimoroni:master Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants