adafruit / Adafruit_CircuitPython_AdafruitIO

Adafruit IO for CircuitPython
http://io.adafruit.com
MIT License
49 stars 33 forks source link

Please use `with` to ensure resource is de-allocated #100

Closed jepler closed 1 year ago

jepler commented 1 year ago

In get, post, etc., the logic is similar to the following:

    def _get(self, path):
        """
        GET data from Adafruit IO

        :param str path: Formatted Adafruit IO URL from _compose_path
        """
        response = self._http.get(
            path, headers=self._create_headers(self._aio_headers[1])
        )
        self._handle_error(response)
        json_data = response.json()
        response.close()
        return json_data

Note that in the case that self._handle_error() throws, the response is not closed.

By using with ... as response:, the response will be automatically closed. The explicit response.close() call can probably be removed too.

I raise this issue because @kattni in discord mentioned encountering an espidf.MemoryError while using Adafruit IO_HTTP to retrieve the current time. However, I don't know if this is actually the cause of her problem.

jepler commented 1 year ago

This only affects the "an error is happening" case. I wrote a test program to run on

Adafruit CircuitPython 8.1.0-beta.2-24-gd2aca7eba0-dirty on 2023-05-05; ESP32-S3-DevKitC-1-N8R2 with ESP32S3

and it has run to 10000 iterations without any MemoryError:

import time
import os
import wifi
import socketpool
import ssl
import adafruit_requests
from adafruit_io.adafruit_io import IO_HTTP

pool = socketpool.SocketPool(wifi.radio)
requests = adafruit_requests.Session(pool, ssl.create_default_context())

io_username = os.getenv('ADAFRUIT_IO_USERNAME')
io_key = os.getenv('ADAFRUIT_IO_KEY')

if not io_username or not io_key:
    raise SystemExit("Set ADAFRUIT_IO_USERNAME and _KEY in settings.toml")
io = IO_HTTP(io_username, io_key, requests)

i = 0
while True:
    i += 1
    print(i, time.monotonic())
    print(io.receive_time())
    #time.sleep(.1)

so while I think this is worth improving, I'm not particular confident it's relevant to Kattni's problem in a larger program.