adafruit / Adafruit_CircuitPython_BME280

CircuitPython driver for the BME280
MIT License
63 stars 41 forks source link

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

Closed kevinjwalters closed 1 year ago

kevinjwalters commented 4 years ago

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:

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

jerryneedell commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

https://github.com/adafruit/circuitpython/issues/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 commented 4 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

Good Will next week . Thanks

jposada202020 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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

jerryneedell commented 3 years ago

Tested and added comments to #52

jerryneedell commented 3 years ago

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

jerryneedell commented 3 years ago

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 commented 2 years ago

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

tekktrik commented 1 year ago

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