Initial Working Version #1

makermelissa · 2020-07-24T23:38:00Z

This uses a combo of the RGB Matrix, RGB Matrix Shield, and Metro M4 Airlift Lite.

ladyada · 2020-07-24T23:46:37Z

@jedgarpark please test for melissa

tannewt

Thanks for this initial work! Two questions:

How much memory is used by this library?
How much of this is duplicated from PyPortal?

The PyPortal library is monolithic and I'd love to see an iteration that better enables loading just the code needed for what someone is trying to do. For example, the SD card and fetching logic could be split from the graphics.

tannewt · 2020-07-27T23:12:55Z

README.rst

+    :alt: Documentation Status
+
+.. image:: https://img.shields.io/discord/327254708534116352.svg
+    :target: https://discord.gg/nBQh6qu


I don't think this invite link is active anymore. Looks like we need to update the cookiecutter.

Ok, I can remove the link at least.

https://github.com/adafruit/Adafruit_CircuitPython_AHTx0/pull/2/files

@dherrada Does cookiecutter need to be updated for the new discord link?

Thanks for the correct link

tannewt · 2020-07-27T23:15:37Z

adafruit_matrixportal.py

+class Fake_Requests:
+    """For faking 'requests' using a local file instead of the network."""
+
+    def __init__(self, filename):
+        self._filename = filename
+        with open(filename, "r") as file:
+            self.text = file.read()
+
+    def json(self):
+        """json parsed version for local requests."""
+        import json  # pylint: disable=import-outside-toplevel
+
+        return json.loads(self.text)
+


Can we split this out into a separate library since it's shared with PyPortal? (I have modifications to this in my PyPortal copy and duplicating it here will leave it behind.)

That's probably a good idea. There's a Blinka version of the PyPortal library as well that also shares this.

makermelissa · 2020-07-28T00:13:54Z

As for how much memory this uses, how would I tell?
Mostly the network stuff is a duplicate of PyPortal.

I went for a less monolithic approach with this library which is why the add_text() function needs to be called after initialization, though I'm sure there's room for improvement. Like I could make it so it doesn't automatically connect to WiFi in case the user just wants to initialize the RGB Matrix, but it kind of defeats the purpose of this library. Also, I'm pretty sure I remove the SD card stuff from here since there's no SD card on a Metro M4 Airlift Lite.

tannewt · 2020-07-28T19:15:10Z

1. As for how much memory this uses, how would I tell?

gc.mem_free() before and after the import is a decent approximation.

2. Mostly the network stuff is a duplicate of PyPortal.

Could we split it out? Some of it does a fetch and writes the result to the FS right? That'd be handy on it's own.

I went for a less monolithic approach with this library which is why the add_text() function needs to be called after initialization, though I'm sure there's room for improvement. Like I could make it so it doesn't automatically connect to WiFi in case the user just wants to initialize the RGB Matrix, but it kind of defeats the purpose of this library. Also, I'm pretty sure I remove the SD card stuff from here since there's no SD card on a Metro M4 Airlift Lite.

I think there needs to be more curation of what is in this library based on what it's purpose is. Now is the time to do it before folks are using it.

I think there are three layers to it that could be split out separately and build on each other.

The "FeatherWing" scenario where the library does board/project specific initialization like connecting the display and esp coprocessor but nothing else.
The graphics layer that has the normal "adafruit quotes" style layout and abstracts away the group stuff.
The highest level that ties in internet URL to the graphics form.

There are a number of other helpers that should be split into their own libraries like wget and fake requests.

makermelissa · 2020-07-28T22:35:47Z

By "FeatherWing" scenario, are you referring to the FeatherWing library and how we handled the board initialization automatically?

For the memory, it looks like it takes about 65K based on before and after the import.

Are you proposing the 3 layers in the same library or different libraries?

makermelissa · 2020-07-29T16:56:52Z

I'm working on splitting this up into multiple files and for the moment will just keep them in the same library. Then we can split them off later without much change other than additional install requirements.

tannewt · 2020-07-29T18:17:32Z

By "FeatherWing" scenario, are you referring to the FeatherWing library and how we handled the board initialization automatically?

Yup exactly.

I'm working on splitting this up into multiple files and for the moment will just keep them in the same library. Then we can split them off later without much change other than additional install requirements.

Thanks! That's a great start.

makermelissa · 2020-08-04T18:57:48Z

I want to verify the examples first. This is close. I'll re-request a review when ready.

makermelissa · 2020-08-04T21:15:35Z

It's ready again for review.

jedgarpark · 2020-08-05T03:50:00Z

This is working great for the On Air sign!

tannewt

A couple small things. Otherwise, it's a good start in splitting it apart. Thanks!

tannewt · 2020-08-06T23:56:21Z

adafruit_matrixportal/network.py

+        now = time.struct_time(
+            (year, month, mday, hours, minutes, seconds, week_day, year_day, is_dst)
+        )
+        print(now)


Suggested change

print(now)

Good catch.

tannewt · 2020-08-06T23:57:59Z

adafruit_matrixportal/wifi.py

+
+    def manager(self, secrets):
+        """Initialize and return the WiFi Manager"""
+        return adafruit_esp32spi_wifimanager.ESPSPI_WiFiManager(self.esp, secrets, None)


Want to cache this instead since you'll call it every IO request?

Sounds good.

tannewt · 2020-08-06T23:59:07Z

adafruit_matrixportal/matrixportal.py

+        :param file_or_color: The filename of the chosen background image, or a hex color.
+
+        """
+        print("Set background to ", file_or_color)


Should these bare prints be behind the _debug attribute?

I think this particular one is more useful regardless of the debug status.

tannewt · 2020-08-06T23:59:47Z

adafruit_matrixportal/fakerequests.py

+
+    def json(self):
+        """json parsed version for local requests."""
+        import json  # pylint: disable=import-outside-toplevel


This can be at the top level now.

Sounds good.

tannewt · 2020-08-07T00:00:20Z

adafruit_matrixportal/fakerequests.py

+        """json parsed version for local requests."""
+        import json  # pylint: disable=import-outside-toplevel
+
+        return json.loads(self.text)


Use json.load here and open the file here as well. That way it never loads the full string into memory.

Although this suggestion will improve the .json() function, I think this will break trying to access the .text property unless json() is called first. I created a text property that loads and returns the text of the file as well. It doesn't store it and use memory, so it may require multiple file reads.

makermelissa · 2020-08-07T01:17:58Z

I made the requested changes. I modified the FakeRequests so that it doesn't store the text, but created a text property so it doesn't break things.

tannewt

Looks good. Thank you!

makermelissa · 2020-08-10T21:17:19Z

Thank you :)

makermelissa added 10 commits July 24, 2020 15:34

Initial Working Commit

f940a31

Added missing requirement

db94e8a

Fixed import order

a3dc7ce

Mocked supervisor module

2873752

Mocked rtc module

7716e89

Mocked rgbmatrix module

8a5c86d

Mocked framebufferio module

a474169

Mocked secrets

f0e681f

Added parameters that mysteriously disappeared

08dd069

Removed irrelevant step since we're not uploading to PyPI

f6e420f

makermelissa requested a review from a team July 24, 2020 23:38

Trying out fix for non-PyPI builds

382cb13

tannewt requested changes Jul 27, 2020

View reviewed changes

Fix discord link in README

6f1d9d4

Fixed Repo Link

a0335ce

makermelissa added 5 commits August 4, 2020 11:27

Refactored everything

12e947a

Fix merge conflict

c2f8240

Fix import order

b32c17b

Updated API doc file for refactored package

91cad1d

Fix bad character in docstring

3523125

Fix values error and update examples

19aa350

makermelissa requested a review from tannewt August 4, 2020 20:16

Remove debug from examples

0f06e5a

makermelissa added 2 commits August 4, 2020 16:30

Some minor changes to make using network module alone more user friendly

19ce2f3

Removed debugging print statement

c2b22f2

tannewt requested changes Aug 7, 2020

View reviewed changes

Made Scott's requested changes

61cb0fb

makermelissa requested a review from tannewt August 7, 2020 01:18

tannewt approved these changes Aug 10, 2020

View reviewed changes

tannewt merged commit 89b4a87 into adafruit:master Aug 10, 2020

Initial Working Version #1

Initial Working Version #1

Uh oh!

Conversation

makermelissa commented Jul 24, 2020

Uh oh!

ladyada commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

makermelissa commented Jul 28, 2020

Uh oh!

tannewt commented Jul 28, 2020

Uh oh!

makermelissa commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

makermelissa commented Jul 29, 2020

Uh oh!

tannewt commented Jul 29, 2020

Uh oh!

makermelissa commented Aug 4, 2020

Uh oh!

makermelissa commented Aug 4, 2020

Uh oh!

jedgarpark commented Aug 5, 2020

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

makermelissa Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

makermelissa commented Aug 7, 2020

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

makermelissa commented Aug 10, 2020

Uh oh!

Uh oh!

ladyada commented Jul 24, 2020 •

edited

Loading

makermelissa commented Jul 28, 2020 •

edited

Loading

makermelissa Aug 7, 2020 •

edited

Loading