Skip to content

Allow for a external session. #505

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

Open
c00w wants to merge 3 commits into
base: main
Choose a base branch
from
+532 −912
Open

Allow for a external session. #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07f0ed3
Allow for a external session.
c00w Jan 31, 2026
440857c
Update tests/conftest.py
firstof9 Feb 2, 2026
2c6cdc2
Fix tests
c00w Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 86 additions & 73 deletions openevsehttp/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,18 @@
class OpenEVSE:
"""Represent an OpenEVSE charger."""

def __init__(self, host: str, user: str = "", pwd: str = "") -> None:
def __init__(
self,
host: str,
user: str = "",
pwd: str = "",
session: aiohttp.ClientSession | None = None,
) -> None:
"""Connect to an OpenEVSE charger equipped with wifi or ethernet."""
self._user = user
self._pwd = pwd
self.url = f"http://{host}/"
self._session = session
self._status: dict = {}
self._config: dict = {}
self._override = None
Expand All @@ -98,6 +105,13 @@ def __init__(self, host: str, user: str = "", pwd: str = "") -> None:
self._loop = None
self.tasks = None

@property
def session(self) -> aiohttp.ClientSession:
"""Return the aiohttp session, creating one if needed."""
if self._session is None:
self._session = aiohttp.ClientSession()
return self._session

async def process_request(
self,
url: str,
Expand All @@ -113,61 +127,57 @@ async def process_request(
if self._user and self._pwd:
auth = aiohttp.BasicAuth(self._user, self._pwd)

async with aiohttp.ClientSession() as session:
http_method = getattr(session, method)
_LOGGER.debug(
"Connecting to %s with data: %s rapi: %s using method %s",
http_method = getattr(self.session, method)
_LOGGER.debug(
"Connecting to %s with data: %s rapi: %s using method %s",
url,
data,
rapi,
method,
)
try:
async with http_method(
url,
data,
rapi,
method,
)
try:
async with http_method(
url,
data=rapi,
json=data,
auth=auth,
) as resp:
try:
message = await resp.text()
except UnicodeDecodeError:
_LOGGER.debug("Decoding error")
message = await resp.read()
message = message.decode(errors="replace")

try:
message = json.loads(message)
except ValueError:
_LOGGER.warning("Non JSON response: %s", message)

if resp.status == 400:
index = ""
if "msg" in message.keys():
index = "msg"
elif "error" in message.keys():
index = "error"
_LOGGER.error("Error 400: %s", message[index])
raise ParseJSONError
if resp.status == 401:
_LOGGER.error("Authentication error: %s", message)
raise AuthenticationError
if resp.status in [404, 405, 500]:
_LOGGER.warning("%s", message)

if method == "post" and "config_version" in message:
await self.update()
return message

except (TimeoutError, ServerTimeoutError) as err:
_LOGGER.error("%s: %s", ERROR_TIMEOUT, url)
raise err
except ContentTypeError as err:
_LOGGER.error("Content error: %s", err.message)
raise err

await session.close()
return message
data=rapi,
json=data,
auth=auth,
) as resp:
try:
message = await resp.text()
except UnicodeDecodeError:
_LOGGER.debug("Decoding error")
message = await resp.read()
message = message.decode(errors="replace")

try:
message = json.loads(message)
except ValueError:
_LOGGER.warning("Non JSON response: %s", message)

if resp.status == 400:
index = ""
if "msg" in message.keys():
index = "msg"
elif "error" in message.keys():
index = "error"
_LOGGER.error("Error 400: %s", message[index])
raise ParseJSONError
if resp.status == 401:
_LOGGER.error("Authentication error: %s", message)
raise AuthenticationError
if resp.status in [404, 405, 500]:
_LOGGER.warning("%s", message)

if method == "post" and "config_version" in message:
await self.update()
return message

except (TimeoutError, ServerTimeoutError) as err:
_LOGGER.error("%s: %s", ERROR_TIMEOUT, url)
raise err
except ContentTypeError as err:
_LOGGER.error("Content error: %s", err.message)
raise err

async def send_command(self, command: str) -> tuple:
"""Send a RAPI command to the charger and parses the response."""
Expand Down Expand Up @@ -204,7 +214,11 @@ async def update(self) -> None:
if not self.websocket:
# Start Websocket listening
self.websocket = OpenEVSEWebsocket(
self.url, self._update_status, self._user, self._pwd
self.url,
self._update_status,
self._user,
self._pwd,
session=self.session,
)

async def test_and_get(self) -> dict:
Expand Down Expand Up @@ -573,23 +587,22 @@ async def firmware_check(self) -> dict | None:
return None

try:
async with aiohttp.ClientSession() as session:
http_method = getattr(session, method)
_LOGGER.debug(
"Connecting to %s using method %s",
url,
method,
)
async with http_method(url) as resp:
if resp.status != 200:
return None
message = await resp.text()
message = json.loads(message)
response = {}
response["latest_version"] = message["tag_name"]
response["release_notes"] = message["body"]
response["release_url"] = message["html_url"]
return response
http_method = getattr(self.session, method)
_LOGGER.debug(
"Connecting to %s using method %s",
url,
method,
)
async with http_method(url) as resp:
if resp.status != 200:
return None
message = await resp.text()
message = json.loads(message)
response = {}
response["latest_version"] = message["tag_name"]
response["release_notes"] = message["body"]
response["release_url"] = message["html_url"]
return response

except (TimeoutError, ServerTimeoutError):
_LOGGER.error("%s: %s", ERROR_TIMEOUT, url)
Expand Down
13 changes: 11 additions & 2 deletions openevsehttp/websocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ def __init__(
callback,
user=None,
password=None,
session=None,
):
"""Initialize a OpenEVSEWebsocket instance."""
self.session = aiohttp.ClientSession()
self._session = session
self.uri = self._get_uri(server)
self._user = user
self._password = password
Expand All @@ -45,6 +46,13 @@ def __init__(
self._ping = None
self._pong = None

@property
def session(self):
"""Return the aiohttp session, creating one if needed."""
if self._session is None:
self._session = aiohttp.ClientSession()
return self._session
Comment on lines +49 to +54
Copy link

@coderabbitai coderabbitai bot Feb 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Get the file structure and size
wc -l openevsehttp/websocket.py

Repository: firstof9/python-openevse-http

Length of output: 102

🏁 Script executed:

# Find all close methods and session cleanup in the file
grep -n "def close\|async def close\|__aexit__\|_session" openevsehttp/websocket.py | head -30

Repository: firstof9/python-openevse-http

Length of output: 262

🏁 Script executed:

# Look for session-related patterns
rg -n "session.*close|close.*session" openevsehttp/ --type py

Repository: firstof9/python-openevse-http

Length of output: 55

🏁 Script executed:

# Check for context manager implementation
rg -n "__aenter__|__aexit__" openevsehttp/ --type py

Repository: firstof9/python-openevse-http

Length of output: 55

🏁 Script executed:

sed -n '30,60p' openevsehttp/websocket.py

Repository: firstof9/python-openevse-http

Length of output: 909

🏁 Script executed:

sed -n '145,165p' openevsehttp/websocket.py

Repository: firstof9/python-openevse-http

Length of output: 880

Track ownership and close internally created sessions.

The session property lazily creates an aiohttp.ClientSession without tracking ownership, but close() only closes _client and never closes _session. If OpenEVSEWebsocket is used directly without injecting a session, the internally created session's connection pool leaks and is never cleaned up. Add an _owns_session flag to track when you create the session internally, and close it only in that case.

Suggested fix 
 class OpenEVSEWebsocket:
     def __init__(
         self,
         server,
         callback,
         user=None,
         password=None,
         session=None,
     ):
         """Initialize a OpenEVSEWebsocket instance."""
         self._session = session
+        self._owns_session = False
 
     `@property`
     def session(self):
         """Return the aiohttp session, creating one if needed."""
         if self._session is None:
             self._session = aiohttp.ClientSession()
+            self._owns_session = True
         return self._session
 
     async def close(self):
         """Close the listening websocket."""
         await OpenEVSEWebsocket.state.fset(self, STATE_STOPPED)
         if self._client:
             await self._client.close()
+        if self._owns_session and self._session and not self._session.closed:
+            await self._session.close()
🤖 Prompt for AI Agents 
In `@openevsehttp/websocket.py` around lines 49 - 54, The session property lazily
creates an aiohttp.ClientSession but doesn't track ownership, causing a
connection-pool leak because close() only closes _client; add an _owns_session
boolean on OpenEVSEWebsocket, set _owns_session = True when session() creates a
new aiohttp.ClientSession and set _owns_session = False when an external session
is injected or passed into __init__, then update the close() method to
await/close self._session only if self._session is not None and
self._owns_session is True (and still close self._client as before); ensure any
assignment paths to self._session set the ownership flag appropriately.

Copy link
Owner

@firstof9 firstof9 Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c00w what do you think of the suggestion proposed here?

Copy link

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


@property
def state(self):
"""Return the current state."""
Expand Down Expand Up @@ -159,7 +167,8 @@ async def listen(self):
async def close(self):
"""Close the listening websocket."""
await self._set_state(STATE_STOPPED)
await self.session.close()
if self._client:
await self._client.close()

async def keepalive(self):
"""Send ping requests to websocket."""
Expand Down
Loading