Skip to content

Remove max_size parameter #45

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2021
Merged

Remove max_size parameter #45

merged 1 commit into from
Jul 5, 2021

Conversation

lesamouraipourpre
Copy link
Contributor

Remove the max_size parameter. It is no longer used by displayio.Group
Ref: adafruit/circuitpython#4959

I don't have a CLUE to test with.

@lesamouraipourpre lesamouraipourpre mentioned this pull request Jul 2, 2021
Closed
41 tasks
@jposada202020
Copy link
Contributor

@lesamouraipourpre
I tested this in

Adafruit CircuitPython 6.3.0 on 2021-06-01; Adafruit CLUE nRF52840 Express with nRF52840

I test the clue_ble_color_patchwork.py

Results with max_size

>>> import clue_ble_color_patchwork
scanning
after scan found 1 results
0
15597568

Results WITHOUT max_size

Adafruit CircuitPython 6.3.0 on 2021-06-01; Adafruit CLUE nRF52840 Express with nRF52840
>>> import clue_ble_color_patchwork
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "clue_ble_color_patchwork.py", line 146, in <module>
  File "/lib/adafruit_display_shapes/rect.py", line 47, in __init__
MemoryError: memory allocation failed, allocating 7680 bytes
>>>

I remember that we were waiting for this change, so not sure what is the consequences memory wise in the core.

@lesamouraipourpre
Copy link
Contributor Author

As far as I can see once it hits the C code below it is simply ignored, so removing it should have no effect.
https://github.com/adafruit/circuitpython/blob/c37f354d2d9faca7bb827c775335421ffa98bf33/shared-bindings/displayio/Group.c#L38-L72

Did you use the .PY version of adafruit_clue.py or a .MPY version? That's the only thing I can think of that could cause a memory overflow.

@jposada202020
Copy link
Contributor

I used the .py version in both cases, just wondering why this will increase the memory usage

@lesamouraipourpre
Copy link
Contributor Author

The .py contains all the comments and is loaded into memory, so depending on the code can be significantly bigger. That's before any of the compression that .mpy applies.

@jposada202020
Copy link
Contributor

Yes, I am aware of that, but I use the py in both cases, maybe something as put the big rock first with the changes. If you agree I will test with the .mpy files and if so, then I could do a different PR to add the note in the example that the . mpy needs to be used in order for the example to work? does this work for you?

@lesamouraipourpre
Copy link
Contributor Author

I'm intrigued as to why this is happening then as there should be a very marginal space saving of a byte or two.

If the .mpy version runs and has the same or better free memory while running I would say merge it and go with your suggestion.

@jposada202020
Copy link
Contributor

yup me too. will do!

@jposada202020
Copy link
Contributor

I have replaced the following libraries for the mpy version and it worked
Bus_device
Register
lsm6ds
apds9960

@jposada202020 jposada202020 merged commit ec30af1 into adafruit:main Jul 5, 2021
@lesamouraipourpre lesamouraipourpre deleted the max-size branch July 6, 2021 09:51
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jul 7, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@5b7e4de)
78cd017 
Updating https://github.com/adafruit/Adafruit_CircuitPython_CLUE to 2.3.0 from 2.2.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_CLUE#46 from jposada202020/adding_note
  > Merge pull request adafruit/Adafruit_CircuitPython_CLUE#45 from lesamouraipourpre/max-size
  > Moved default branch to main
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template
  > "Increase duplicate code check threshold "

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Button to 1.6.0 from 1.5.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Button#32 from lesamouraipourpre/requirements
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Button#31 from lesamouraipourpre/max-size
  > Moved default branch to main
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template

Updating https://github.com/adafruit/Adafruit_CircuitPython_ProgressBar to 2.3.0 from 2.2.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_ProgressBar#32 from lesamouraipourpre/max-size
  > Moved default branch to main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jposada202020 jposada202020 jposada202020 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lesamouraipourpre @jposada202020