Skip to content

Minor reduction in memory footprint to make library more viable with adafruit_rfm9x on M0 processors #37

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

Closed
kevinjwalters opened this issue Jun 12, 2020 · 18 comments

Comments

@kevinjwalters
Copy link

kevinjwalters commented Jun 12, 2020

This library is used in the guide article, Adafruit Learn: Feather + Raspberry Pi Weather Monitoring Network with LoRa or LoRaWAN. It is being discussed in Adafruit Forums: Memory Error Lora_Device with BME280 as the CircuitPython code in the guide hits a MemoryError.

File "code.py", line 15, in <module>
File "adafruit_bme280.py", line 499, in <module>
File "adafruit_bme280.py", line 517, in Adafruit_BME280_SPI
MemoryError:

If everything is working as expected here then shrinking adafruit_rfm9x or adafruit_bme280 are the only options to get this working.

Some ideas:

  • The raise ValueError in all the setters currently have a specific message per property. This could be made generic and then six strings get condensed into one.
  • You could try putting the i2c/spi classes in separate files but I've got a vague feeling this would make it worse.
  • For the private methods with underscore prefixes (i.e. ones not part of the interface which is already in use by applications) you could abbreviate them more as these end repeatedly in the mpy. If desperation reaches this stage I'd verify this has some effect with a gc.collect() ; print(mem_free()).

@brentru and @jerryneedell might have some other or better ideas on this.

@jerryneedell
Copy link
Contributor

jerryneedell commented Jun 12, 2020

I'll try that -- also , does the use of properties (getters/setters) add a lot to the code size. I'm still trying to figure out why this is so big. I took out the use of "warnings" that saved a few hundred bytes.

edited to ad -- I am working on the rfm9x lib -- not the bme280 lib

@kevinjwalters
Copy link
Author

kevinjwalters commented Jun 12, 2020

I also note the example code on https://learn.adafruit.com/multi-device-lora-temperature-network/using-with-adafruitio#feather-code-usage-3017649-1 has this at lines 78-83:

    # Convert bytearray to bytes
    bme280_data_bytes = bytes(bme280_data)
    # Send the packet data
    print('Sending data...')
    LED.value = True
    rfm9x.send(bme280_data)

bme280_data_bytes is unused in the code. Worth a quick review of this and running pylint over this to check for other easy savings.

If things are desperate I would also try experimenting with the non-frozen library order in example code, e.g. swapping aroud the order of the two adafruit_* libraries. This can have a substantial effect on memory usage for some reason. It may also make things worse so a bit of temporary printing of gc.collect() ; print(gc.mem_free()) afterwards to check is wise.

@kevinjwalters
Copy link
Author

kevinjwalters commented Jun 12, 2020

I think properties do add to the library size. I initially had channel in MIDIMessage from the adafruit_midi library just as an instance variable but did eventually turn this into a property to be able to range check it. There's a constant trade-off here between functionality/sophistication/helpful error/robustness and code size.

I have wondered about libraries with some sort of conditional features for boards bigger than M0 where the M0 one would be trimmed back. As an analogy, a C programmmer might do this with preprocessor to generate different versions. This does create a whole set of separate library management issues with selection and documentation.

@kevinjwalters
Copy link
Author

Very crude summary of symbols in the current (from 19-May-2020 bundle) mpy.

$ strings adafruit-circuitpython-bundle-5.x-mpy-20200519/lib/adafruit_bme280.mpy | grep '^_' | sort | uniq -c | sort -rbn
     13 _humidity_calib
     11 _read_register
     10 _write_register_byte
     10 _pressure_calib
     10 _BME280_OVERSCANS
      8 __init__
      7 _write_ctrl_meas
      6 _temp_calib
      6 _read_byte
      5 _write_config
      5 _t_standby
      5 _t_fine
      5 _read_temperature
      5 _iir_filter
      4 _read24
      4 _mode
      3 _spi
      3 _reset
      3 _read_coefficients
      3 __qualname__
      3 _overscan_temperature
      3 _overscan_pressure
      3 _overscan_humidity
      3 __name__
      3 __module__
      3 _i2c
      3 _get_status
      3 _ctrl_meas
      3 _config
      2 _read_config
      2 _BME280_STANDBY_TCS
      2 _BME280_MODES
      2 _BME280_IIR_FILTERS
      1 __version__
      1 __repo__

@jerryneedell
Copy link
Contributor

jerryneedell commented Jun 12, 2020

FYI -- for the rfm9x lib I get

jerryneedell@jerryneedell-ubuntu-macmini:~/projects/adafruit_github/Adafruit_CircuitPython_RFM9x$ strings ~/bundle/5.x/adafruit-circuitpython-bundle-5.x-mpy-20200526/lib/adafruit_rfm9x.mpy | grep '^_' | sort | uniq -c | sort -rbn
     20 _read_u8
     16 _RegisterBits
     10 _BUFFER
      9 _mask
      4 _reset
      4 __init__
      4 _device
      4 _address
      3 _read_into
      3 __name__
      2 _write_from	_write_u8
      2 __set__
      2 _RH_RF95_FXOSC
      2 _RH_RF95_FSTEP
      2 _offset
      2 __module__
      2 __get__
      2 _configure_radio
      1 _write_from
      1 __version__
      1 _RH_RF95_FSTEP	_write_u8	_write_u8	_write_u8
      1 __repo__
      1 _read_u8	_write_u8	bytearray
      1 _read_u8	_write_u8
      1 _read_into	_write_u8
      1 __qualname__	bytearray
      1 __qualname__
      1 _offset	_write_u8
      1 _'',i&%G%%L7%%
      1 _configure_radio	last_rssi

@kevinjwalters
Copy link
Author

kevinjwalters commented Jun 15, 2020

adafruit/circuitpython#145 is interesting. I wonder if that would help? Even if code.mpy is not supported then a code.py with the bulk of the code moved out to a program.mpy might help here?

@kevinjwalters
Copy link
Author

kevinjwalters commented Jun 15, 2020

@jepler also mentioned that stack size is configurable in boot.py https://circuitpython.readthedocs.io/en/5.0.x/shared-bindings/supervisor/__init__.html?highlight=stack#supervisor.set_next_stack_limit

Origins of this from Discord:

the stack size increase for the CPX library was needed because of a 5 or 6-level-deep call chain during init(). init() for the main class gets called during import because of the assignment to the singleton cp.
[14:46] danh: We had this assignment before, but the addition of a subclass in the CPX library and I think some additional nesting in some device libraries caused the stack overflow

@jposada202020
Copy link
Contributor

@tannewt should we try something similar here..to the work done in the DPS310? willing to give it a try, let me know. Thanks

@tannewt
Copy link
Member

tannewt commented May 20, 2021

@tannewt should we try something similar here..to the work done in the DPS310? willing to give it a try, let me know. Thanks

Sure!

@jposada202020
Copy link
Contributor

Good Will next week . Thanks

@jposada202020
Copy link
Contributor

@jerryneedell could you try this PR #52
Disclosure: I tried but could not make all the libraries to fit. But you folks seems to achieve that in the first place so that is why I am asking. You probably should use basic.mpy on the PR. Thanks :)

@jerryneedell
Copy link
Contributor

@jerryneedell could you try this PR #52
Disclosure: I tried but could not make all the libraries to fit. But you folks seems to achieve that in the first place so that is why I am asking. You probably should use basic.mpy on the PR. Thanks :)

Sure -- I'll give it a try later today or tomorrow.

@jposada202020
Copy link
Contributor

Thanks :) ... is the only one that I could not test and the reason for that PR :)

@jerryneedell
Copy link
Contributor

Tested and added comments to #52

@jerryneedell
Copy link
Contributor

Ah -- I did not complete the test to use the radio at the same time -- will do that now

@jerryneedell
Copy link
Contributor

Ran the demo from https://learn.adafruit.com/multi-device-lora-temperature-network/using-with-adafruitio using basic:
looks good!


Adafruit CircuitPython 7.0.0-alpha.2-705-gc80e25307 on 2021-05-30; Adafruit Feather M0 RFM9x with samd21g18
>>> 
>>> import rfm_test

Temperature: 23.7 C
Humidity: 42.2 %
Pressure: 1024.0 hPa
Sending data...
Sent data!

@tekktrik
Copy link
Member

Was perusing open issues, it looks like this one is complete, correct? :)

@tekktrik
Copy link
Member

Closing because it looks like the original issue was solved via #52, but please reopen if I'm mistaken!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants