From 38fe04937dd4f93183112e74124e77e33bebfe3a Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 09:46:22 +0200 Subject: [PATCH 01/19] Improve write_cache() function, suggested by CRAI --- plugwise_usb/helpers/cache.py | 110 +++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 40 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index ad11b70d3..5d9f0fb40 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -79,46 +79,76 @@ def _get_writable_os_dir(self) -> str: ) return os_path_join(os_path_expand_user("~"), CACHE_DIR) - async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: - """Save information to cache file.""" - if not self._initialized: - raise CacheError( - f"Unable to save cache. Initialize cache file '{self._file_name}' first." - ) - - current_data: dict[str, str] = {} - if not rewrite: - current_data = await self.read_cache() - processed_keys: list[str] = [] - data_to_write: list[str] = [] - for _cur_key, _cur_val in current_data.items(): - _write_val = _cur_val - if _cur_key in data: - _write_val = data[_cur_key] - processed_keys.append(_cur_key) - data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") - # Write remaining new data - for _key, _value in data.items(): - if _key not in processed_keys: - data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") - - try: - async with aiofiles_open( - file=self._cache_file, - mode="w", - encoding=UTF8, - ) as file_data: - await file_data.writelines(data_to_write) - except OSError as exc: - _LOGGER.warning( - "%s while writing data to cache file %s", exc, str(self._cache_file) - ) - else: - if not self._cache_file_exists: - self._cache_file_exists = True - _LOGGER.debug( - "Saved %s lines to cache file %s", str(len(data)), self._cache_file - ) +async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: + """Save information to cache file atomically using aiofiles + temp file.""" + if not self._initialized: + raise CacheError( + f"Unable to save cache. Initialize cache file '{self._file_name}' first." + ) + + current_data: dict[str, str] = {} + if not rewrite: + current_data = await self.read_cache() + + processed_keys: list[str] = [] + data_to_write: list[str] = [] + + # Prepare data exactly as in original implementation + for _cur_key, _cur_val in current_data.items(): + _write_val = _cur_val + if _cur_key in data: + _write_val = data[_cur_key] + processed_keys.append(_cur_key) + data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") + + # Write remaining new data + for _key, _value in data.items(): + if _key not in processed_keys: + data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") + + # Atomic write using aiofiles with temporary file + cache_file_path = Path(self._cache_file) + temp_path = cache_file_path.with_name(f".{cache_file_path.name}.tmp.{os.getpid()}") + + try: + # Write to temporary file using aiofiles + async with aiofiles_open( + file=str(temp_path), + mode="w", + encoding=UTF8, + ) as temp_file: + await temp_file.writelines(data_to_write) + await temp_file.flush() + + # Atomic rename + if os.name == 'nt' and cache_file_path.exists(): + cache_file_path.unlink() + + temp_path.replace(cache_file_path) + temp_path = None # Successfully renamed + + if not self._cache_file_exists: + self._cache_file_exists = True + + _LOGGER.debug( + "Saved %s lines to cache file %s (aiofiles atomic write)", + str(len(data)), + self._cache_file + ) + + except OSError as exc: + _LOGGER.warning( + "%s while writing data to cache file %s (aiofiles atomic write)", + exc, + str(self._cache_file) + ) + finally: + # Cleanup on error + if temp_path and temp_path.exists(): + try: + temp_path.unlink() + except OSError: + pass async def read_cache(self) -> dict[str, str]: """Return current data from cache file.""" From 7ee83c7fefaccfe1ad65d2361e5826a7a82afe68 Mon Sep 17 00:00:00 2001 From: autoruff Date: Mon, 15 Sep 2025 07:47:02 +0000 Subject: [PATCH 02/19] fixup: improve-cache-writing Python code reformatted using Ruff --- plugwise_usb/helpers/cache.py | 33 +++++++++++++++++---------------- plugwise_usb/network/cache.py | 8 ++++++-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 5d9f0fb40..141af6f6a 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -79,6 +79,7 @@ def _get_writable_os_dir(self) -> str: ) return os_path_join(os_path_expand_user("~"), CACHE_DIR) + async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: """Save information to cache file atomically using aiofiles + temp file.""" if not self._initialized: @@ -89,10 +90,10 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None current_data: dict[str, str] = {} if not rewrite: current_data = await self.read_cache() - + processed_keys: list[str] = [] data_to_write: list[str] = [] - + # Prepare data exactly as in original implementation for _cur_key, _cur_val in current_data.items(): _write_val = _cur_val @@ -100,7 +101,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None _write_val = data[_cur_key] processed_keys.append(_cur_key) data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") - + # Write remaining new data for _key, _value in data.items(): if _key not in processed_keys: @@ -109,7 +110,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None # Atomic write using aiofiles with temporary file cache_file_path = Path(self._cache_file) temp_path = cache_file_path.with_name(f".{cache_file_path.name}.tmp.{os.getpid()}") - + try: # Write to temporary file using aiofiles async with aiofiles_open( @@ -119,28 +120,28 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None ) as temp_file: await temp_file.writelines(data_to_write) await temp_file.flush() - + # Atomic rename - if os.name == 'nt' and cache_file_path.exists(): + if os.name == "nt" and cache_file_path.exists(): cache_file_path.unlink() - + temp_path.replace(cache_file_path) temp_path = None # Successfully renamed - + if not self._cache_file_exists: self._cache_file_exists = True - + _LOGGER.debug( - "Saved %s lines to cache file %s (aiofiles atomic write)", - str(len(data)), - self._cache_file + "Saved %s lines to cache file %s (aiofiles atomic write)", + str(len(data)), + self._cache_file, ) - + except OSError as exc: _LOGGER.warning( - "%s while writing data to cache file %s (aiofiles atomic write)", - exc, - str(self._cache_file) + "%s while writing data to cache file %s (aiofiles atomic write)", + exc, + str(self._cache_file), ) finally: # Cleanup on error diff --git a/plugwise_usb/network/cache.py b/plugwise_usb/network/cache.py index 3f6d4a3f8..06e672ccb 100644 --- a/plugwise_usb/network/cache.py +++ b/plugwise_usb/network/cache.py @@ -30,7 +30,9 @@ async def save_cache(self) -> None: mac: node_type.name for mac, node_type in self._nodetypes.items() } _LOGGER.debug("Save NodeTypes for %s Nodes", len(cache_data_to_save)) - await self.write_cache(cache_data_to_save, rewrite=True) # Make sure the cache-contents is actual + await self.write_cache( + cache_data_to_save, rewrite=True + ) # Make sure the cache-contents is actual async def clear_cache(self) -> None: """Clear current cache.""" @@ -54,7 +56,9 @@ async def restore_cache(self) -> None: node_type = None if node_type is None: - _LOGGER.warning("Invalid NodeType in cache for mac %s: %s", mac, node_value) + _LOGGER.warning( + "Invalid NodeType in cache for mac %s: %s", mac, node_value + ) continue self._nodetypes[mac] = node_type _LOGGER.debug( From 450e64ce8130b7ec5f82a36d7d2da91421da909d Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 09:49:56 +0200 Subject: [PATCH 03/19] Fix missing import --- plugwise_usb/helpers/cache.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 141af6f6a..b1b972780 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -6,6 +6,7 @@ import logging from os import getenv as os_getenv, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join +from pathlib import Path from aiofiles import open as aiofiles_open, ospath # type: ignore[import-untyped] from aiofiles.os import ( # type: ignore[import-untyped] From e7e6056c5d52077ea5b31642a2f9bb7bd2a3e650 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 09:53:23 +0200 Subject: [PATCH 04/19] Fix indent --- plugwise_usb/helpers/cache.py | 138 +++++++++++++++++----------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index b1b972780..0c5d816cf 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -81,76 +81,76 @@ def _get_writable_os_dir(self) -> str: return os_path_join(os_path_expand_user("~"), CACHE_DIR) -async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: - """Save information to cache file atomically using aiofiles + temp file.""" - if not self._initialized: - raise CacheError( - f"Unable to save cache. Initialize cache file '{self._file_name}' first." - ) - - current_data: dict[str, str] = {} - if not rewrite: - current_data = await self.read_cache() - - processed_keys: list[str] = [] - data_to_write: list[str] = [] - - # Prepare data exactly as in original implementation - for _cur_key, _cur_val in current_data.items(): - _write_val = _cur_val - if _cur_key in data: - _write_val = data[_cur_key] - processed_keys.append(_cur_key) - data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") - - # Write remaining new data - for _key, _value in data.items(): - if _key not in processed_keys: - data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") - - # Atomic write using aiofiles with temporary file - cache_file_path = Path(self._cache_file) - temp_path = cache_file_path.with_name(f".{cache_file_path.name}.tmp.{os.getpid()}") - - try: - # Write to temporary file using aiofiles - async with aiofiles_open( - file=str(temp_path), - mode="w", - encoding=UTF8, - ) as temp_file: - await temp_file.writelines(data_to_write) - await temp_file.flush() - - # Atomic rename - if os.name == "nt" and cache_file_path.exists(): - cache_file_path.unlink() - - temp_path.replace(cache_file_path) - temp_path = None # Successfully renamed + async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: + """Save information to cache file atomically using aiofiles + temp file.""" + if not self._initialized: + raise CacheError( + f"Unable to save cache. Initialize cache file '{self._file_name}' first." + ) - if not self._cache_file_exists: - self._cache_file_exists = True - - _LOGGER.debug( - "Saved %s lines to cache file %s (aiofiles atomic write)", - str(len(data)), - self._cache_file, - ) - - except OSError as exc: - _LOGGER.warning( - "%s while writing data to cache file %s (aiofiles atomic write)", - exc, - str(self._cache_file), - ) - finally: - # Cleanup on error - if temp_path and temp_path.exists(): - try: - temp_path.unlink() - except OSError: - pass + current_data: dict[str, str] = {} + if not rewrite: + current_data = await self.read_cache() + + processed_keys: list[str] = [] + data_to_write: list[str] = [] + + # Prepare data exactly as in original implementation + for _cur_key, _cur_val in current_data.items(): + _write_val = _cur_val + if _cur_key in data: + _write_val = data[_cur_key] + processed_keys.append(_cur_key) + data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") + + # Write remaining new data + for _key, _value in data.items(): + if _key not in processed_keys: + data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") + + # Atomic write using aiofiles with temporary file + cache_file_path = Path(self._cache_file) + temp_path = cache_file_path.with_name(f".{cache_file_path.name}.tmp.{os.getpid()}") + + try: + # Write to temporary file using aiofiles + async with aiofiles_open( + file=str(temp_path), + mode="w", + encoding=UTF8, + ) as temp_file: + await temp_file.writelines(data_to_write) + await temp_file.flush() + + # Atomic rename + if os.name == "nt" and cache_file_path.exists(): + cache_file_path.unlink() + + temp_path.replace(cache_file_path) + temp_path = None # Successfully renamed + + if not self._cache_file_exists: + self._cache_file_exists = True + + _LOGGER.debug( + "Saved %s lines to cache file %s (aiofiles atomic write)", + str(len(data)), + self._cache_file, + ) + + except OSError as exc: + _LOGGER.warning( + "%s while writing data to cache file %s (aiofiles atomic write)", + exc, + str(self._cache_file), + ) + finally: + # Cleanup on error + if temp_path and temp_path.exists(): + try: + temp_path.unlink() + except OSError: + pass async def read_cache(self) -> dict[str, str]: """Return current data from cache file.""" From 9c0dd05eb755ce14b1361542d3a5f9f7f1a44a97 Mon Sep 17 00:00:00 2001 From: autoruff Date: Mon, 15 Sep 2025 07:53:46 +0000 Subject: [PATCH 05/19] fixup: improve-cache-writing Python code reformatted using Ruff --- plugwise_usb/helpers/cache.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 0c5d816cf..684f6577f 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -80,7 +80,6 @@ def _get_writable_os_dir(self) -> str: ) return os_path_join(os_path_expand_user("~"), CACHE_DIR) - async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: """Save information to cache file atomically using aiofiles + temp file.""" if not self._initialized: @@ -110,7 +109,9 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None # Atomic write using aiofiles with temporary file cache_file_path = Path(self._cache_file) - temp_path = cache_file_path.with_name(f".{cache_file_path.name}.tmp.{os.getpid()}") + temp_path = cache_file_path.with_name( + f".{cache_file_path.name}.tmp.{os.getpid()}" + ) try: # Write to temporary file using aiofiles From 88ff31eb2a0cb03bade64d66b9c4f2e0ecf597bd Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 09:58:44 +0200 Subject: [PATCH 06/19] Fix another missing import --- plugwise_usb/helpers/cache.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 684f6577f..1eeb1ee5f 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -4,7 +4,7 @@ from asyncio import get_running_loop import logging -from os import getenv as os_getenv, name as os_name +from os import getenv as os_getenv, getpid as os_getpid, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path @@ -110,7 +110,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None # Atomic write using aiofiles with temporary file cache_file_path = Path(self._cache_file) temp_path = cache_file_path.with_name( - f".{cache_file_path.name}.tmp.{os.getpid()}" + f".{cache_file_path.name}.tmp.{os_getpid()}" ) try: @@ -124,7 +124,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None await temp_file.flush() # Atomic rename - if os.name == "nt" and cache_file_path.exists(): + if os_name == "nt" and cache_file_path.exists(): cache_file_path.unlink() temp_path.replace(cache_file_path) From b3d6f92747a6f88f5291874a3ceb14123eea103c Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 10:07:01 +0200 Subject: [PATCH 07/19] Ruff fix --- plugwise_usb/helpers/cache.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 1eeb1ee5f..fe9a20636 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -4,6 +4,7 @@ from asyncio import get_running_loop import logging +from contextlib import suppress from os import getenv as os_getenv, getpid as os_getpid, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path @@ -148,10 +149,8 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None finally: # Cleanup on error if temp_path and temp_path.exists(): - try: + with suppress(OSError): temp_path.unlink() - except OSError: - pass async def read_cache(self) -> dict[str, str]: """Return current data from cache file.""" From 57f8fefef20b204016bed8cf9ad00686f78053a6 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 10:18:32 +0200 Subject: [PATCH 08/19] More fixes --- plugwise_usb/helpers/cache.py | 40 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index fe9a20636..ea4fd4ab9 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -3,8 +3,8 @@ from __future__ import annotations from asyncio import get_running_loop -import logging from contextlib import suppress +import logging from os import getenv as os_getenv, getpid as os_getpid, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path @@ -109,10 +109,11 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") # Atomic write using aiofiles with temporary file - cache_file_path = Path(self._cache_file) - temp_path = cache_file_path.with_name( - f".{cache_file_path.name}.tmp.{os_getpid()}" - ) + if self._cache_file is not None: + cache_file_path = Path(self._cache_file) + temp_path = cache_file_path.with_name( + f".{cache_file_path.name}.tmp.{os_getpid()}" + ) try: # Write to temporary file using aiofiles @@ -165,19 +166,21 @@ async def read_cache(self) -> dict[str, str]: self._cache_file, ) return current_data - try: - async with aiofiles_open( - file=self._cache_file, - encoding=UTF8, - ) as read_file_data: - lines: list[str] = await read_file_data.readlines() - except OSError as exc: - # suppress file errors as this is expected the first time - # when no cache file exists yet. - _LOGGER.warning( - "OS error %s while reading cache file %s", exc, str(self._cache_file) - ) - return current_data + + if self._cache_file is not None: + try: + async with aiofiles_open( + file=self._cache_file, + encoding=UTF8, + ) as read_file_data: + lines: list[str] = await read_file_data.readlines() + except OSError as exc: + # suppress file errors as this is expected the first time + # when no cache file exists yet. + _LOGGER.warning( + "OS error %s while reading cache file %s", exc, str(self._cache_file) + ) + return current_data for line in lines: data = line.strip() @@ -189,6 +192,7 @@ async def read_cache(self) -> dict[str, str]: ) break current_data[data[:index_separator]] = data[index_separator + 1 :] + return current_data async def delete_cache(self) -> None: From 8004b6da8e1b46af50689493ae8e35e0eec905dd Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 10:37:03 +0200 Subject: [PATCH 09/19] Improve --- plugwise_usb/helpers/cache.py | 43 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index ea4fd4ab9..eb2ce8f26 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -109,11 +109,13 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None data_to_write.append(f"{_key}{CACHE_KEY_SEPARATOR}{_value}\n") # Atomic write using aiofiles with temporary file - if self._cache_file is not None: - cache_file_path = Path(self._cache_file) - temp_path = cache_file_path.with_name( - f".{cache_file_path.name}.tmp.{os_getpid()}" - ) + if self._cache_file is None: + raise CacheError("Unable to save cache, cache-file has no name") + + cache_file_path = Path(self._cache_file) + temp_path = cache_file_path.with_name( + f".{cache_file_path.name}.tmp.{os_getpid()}" + ) try: # Write to temporary file using aiofiles @@ -160,6 +162,10 @@ async def read_cache(self) -> dict[str, str]: f"Unable to save cache. Initialize cache file '{self._file_name}' first." ) current_data: dict[str, str] = {} + if self._cache_file is None: + _LOGGER.debug("Cache file has no name, return empty cache data") + return current_data + if not self._cache_file_exists: _LOGGER.debug( "Cache file '%s' does not exists, return empty cache data", @@ -167,20 +173,19 @@ async def read_cache(self) -> dict[str, str]: ) return current_data - if self._cache_file is not None: - try: - async with aiofiles_open( - file=self._cache_file, - encoding=UTF8, - ) as read_file_data: - lines: list[str] = await read_file_data.readlines() - except OSError as exc: - # suppress file errors as this is expected the first time - # when no cache file exists yet. - _LOGGER.warning( - "OS error %s while reading cache file %s", exc, str(self._cache_file) - ) - return current_data + try: + async with aiofiles_open( + file=self._cache_file, + encoding=UTF8, + ) as read_file_data: + lines: list[str] = await read_file_data.readlines() + except OSError as exc: + # suppress file errors as this is expected the first time + # when no cache file exists yet. + _LOGGER.warning( + "OS error %s while reading cache file %s", exc, str(self._cache_file) + ) + return current_data for line in lines: data = line.strip() From 9cc0cef09beb0d3c58676c95e1cb43cdf293f779 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 11:09:04 +0200 Subject: [PATCH 10/19] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73db66ed6..68744d125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - PR [337](https://github.com/plugwise/python-plugwise-usb/pull/337): Improve node removal, remove and reset the node as executed by Source, and remove the cache-file. - PR [342](https://github.com/plugwise/python-plugwise-usb/pull/342): Improve node_type chaching. +- PR [343](https://github.com/plugwise/python-plugwise-usb/pull/343): Improve writing of cache-files. ## 0.46.0 - 2025-09-12 From 181f4afe9cdee186c3fbb025312cffe352ba4595 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 11:10:49 +0200 Subject: [PATCH 11/19] Bump version to v0.46.1a2 for testing --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 524d50398..b2db8aace 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "plugwise_usb" -version = "0.46.1a1" +version = "0.46.1a2" license = "MIT" keywords = ["home", "automation", "plugwise", "module", "usb"] classifiers = [ From 6dff29e3ca09c33693b0a44b3c9affb06aa79371 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 13:38:28 +0200 Subject: [PATCH 12/19] Final CRAI improvements --- plugwise_usb/helpers/cache.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index eb2ce8f26..0d772f128 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -4,8 +4,9 @@ from asyncio import get_running_loop from contextlib import suppress +from secrets import token_hex as secrets_token_hex import logging -from os import getenv as os_getenv, getpid as os_getpid, name as os_name +from os import fsync as os_fsync, getenv as os_getenv, getpid as os_getpid, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path @@ -56,7 +57,7 @@ async def initialize_cache(self, create_root_folder: bool = False) -> None: if self._root_dir != "": if not create_root_folder and not await ospath.exists(self._root_dir): raise CacheError( - f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exists." + f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exist." ) cache_dir = self._root_dir else: @@ -114,7 +115,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None cache_file_path = Path(self._cache_file) temp_path = cache_file_path.with_name( - f".{cache_file_path.name}.tmp.{os_getpid()}" + f".{cache_file_path.name}.tmp.{os_getpid()}.{secrets_token_hex(8)}" ) try: @@ -126,11 +127,11 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None ) as temp_file: await temp_file.writelines(data_to_write) await temp_file.flush() + # Ensure data reaches disk before rename + loop = get_running_loop() + await loop.run_in_executor(None, os_fsync, temp_file.fileno()) - # Atomic rename - if os_name == "nt" and cache_file_path.exists(): - cache_file_path.unlink() - + # Atomic rename (overwrites atomically on all platforms) temp_path.replace(cache_file_path) temp_path = None # Successfully renamed @@ -139,7 +140,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None _LOGGER.debug( "Saved %s lines to cache file %s (aiofiles atomic write)", - str(len(data)), + len(data_to_write), self._cache_file, ) @@ -168,7 +169,7 @@ async def read_cache(self) -> dict[str, str]: if not self._cache_file_exists: _LOGGER.debug( - "Cache file '%s' does not exists, return empty cache data", + "Cache file '%s' does not exist, return empty cache data", self._cache_file, ) return current_data @@ -195,7 +196,8 @@ async def read_cache(self) -> dict[str, str]: data, str(self._cache_file), ) - break + continue + current_data[data[:index_separator]] = data[index_separator + 1 :] return current_data From 14a382b1ec8e59dac33f7f1417f2df004b5c380e Mon Sep 17 00:00:00 2001 From: autoruff Date: Mon, 15 Sep 2025 11:38:55 +0000 Subject: [PATCH 13/19] fixup: improve-cache-writing Python code reformatted using Ruff --- plugwise_usb/helpers/cache.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 0d772f128..605b2135e 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -6,7 +6,12 @@ from contextlib import suppress from secrets import token_hex as secrets_token_hex import logging -from os import fsync as os_fsync, getenv as os_getenv, getpid as os_getpid, name as os_name +from os import ( + fsync as os_fsync, + getenv as os_getenv, + getpid as os_getpid, + name as os_name, +) from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path From 244fa0ef8506b4cbf0b572740e72de8abaa0c276 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 14:00:37 +0200 Subject: [PATCH 14/19] Sort imports --- plugwise_usb/helpers/cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 605b2135e..a92aff80e 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -4,7 +4,6 @@ from asyncio import get_running_loop from contextlib import suppress -from secrets import token_hex as secrets_token_hex import logging from os import ( fsync as os_fsync, @@ -14,6 +13,7 @@ ) from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path +from secrets import token_hex as secrets_token_hex from aiofiles import open as aiofiles_open, ospath # type: ignore[import-untyped] from aiofiles.os import ( # type: ignore[import-untyped] From 89ee9ada79c9171dd0c21fbf056517af0c2ef772 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 14:53:15 +0200 Subject: [PATCH 15/19] CRAI suggestions --- plugwise_usb/helpers/cache.py | 37 +++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index a92aff80e..86771f396 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -6,10 +6,13 @@ from contextlib import suppress import logging from os import ( + O_RDONLY as os_O_RDONLY, + close as os_close, fsync as os_fsync, getenv as os_getenv, getpid as os_getpid, name as os_name, + open as os_open, ) from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path @@ -27,6 +30,15 @@ _LOGGER = logging.getLogger(__name__) +def _fsync_parent_dir(path: Path) -> None: + """Ensure persistence on POSIX.""" + fd = os_open(str(path), os_O_RDONLY) + try: + os_fsync(fd) + finally: + os_close(fd) + + class PlugwiseCache: """Base class to cache plugwise information.""" @@ -87,7 +99,9 @@ def _get_writable_os_dir(self) -> str: ) return os_path_join(os_path_expand_user("~"), CACHE_DIR) - async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: + async def write_cache( + self, data: dict[str, str], rewrite: bool = False + ) -> None: # noqa: PLR0912 """Save information to cache file atomically using aiofiles + temp file.""" if not self._initialized: raise CacheError( @@ -98,7 +112,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None if not rewrite: current_data = await self.read_cache() - processed_keys: list[str] = [] + processed_keys: set[str] = set() data_to_write: list[str] = [] # Prepare data exactly as in original implementation @@ -106,7 +120,7 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None _write_val = _cur_val if _cur_key in data: _write_val = data[_cur_key] - processed_keys.append(_cur_key) + processed_keys.add(_cur_key) data_to_write.append(f"{_cur_key}{CACHE_KEY_SEPARATOR}{_write_val}\n") # Write remaining new data @@ -129,16 +143,27 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None file=str(temp_path), mode="w", encoding=UTF8, + newline="\n", ) as temp_file: await temp_file.writelines(data_to_write) await temp_file.flush() # Ensure data reaches disk before rename - loop = get_running_loop() - await loop.run_in_executor(None, os_fsync, temp_file.fileno()) + try: + loop = get_running_loop() + await loop.run_in_executor(None, os_fsync, temp_file.fileno()) + except (OSError, TypeError, AttributeError): + # If fsync fails due to fileno() issues or other problems, + # continue without it. flush() provides reasonable durability. + pass # Atomic rename (overwrites atomically on all platforms) temp_path.replace(cache_file_path) temp_path = None # Successfully renamed + if os_name != "nt": + # Ensure directory entry is persisted on POSIX + await loop.run_in_executor( + None, _fsync_parent_dir, cache_file_path.parent + ) if not self._cache_file_exists: self._cache_file_exists = True @@ -165,7 +190,7 @@ async def read_cache(self) -> dict[str, str]: """Return current data from cache file.""" if not self._initialized: raise CacheError( - f"Unable to save cache. Initialize cache file '{self._file_name}' first." + f"Unable to read cache. Initialize cache file '{self._file_name}' first." ) current_data: dict[str, str] = {} if self._cache_file is None: From 27188a733d0087c2a9e2c24593e014aa1ba84a0b Mon Sep 17 00:00:00 2001 From: autoruff Date: Mon, 15 Sep 2025 13:14:18 +0000 Subject: [PATCH 16/19] fixup: improve-cache-writing Python code reformatted using Ruff --- plugwise_usb/helpers/cache.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 86771f396..7b16bd34f 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -99,9 +99,7 @@ def _get_writable_os_dir(self) -> str: ) return os_path_join(os_path_expand_user("~"), CACHE_DIR) - async def write_cache( - self, data: dict[str, str], rewrite: bool = False - ) -> None: # noqa: PLR0912 + async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: # noqa: PLR0912 """Save information to cache file atomically using aiofiles + temp file.""" if not self._initialized: raise CacheError( From 81584c7cdbd7453f92730bd35869f820bcd32eb0 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 15:32:44 +0200 Subject: [PATCH 17/19] One more CRAI improvement --- plugwise_usb/helpers/cache.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 7b16bd34f..8046777b2 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -159,9 +159,14 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None temp_path = None # Successfully renamed if os_name != "nt": # Ensure directory entry is persisted on POSIX - await loop.run_in_executor( - None, _fsync_parent_dir, cache_file_path.parent - ) + try: + await loop.run_in_executor( + None, _fsync_parent_dir, cache_file_path.parent + ) + except (OSError, PermissionError): + # Directory fsync may fail on some filesystems + # The atomic rename is still complete + pass if not self._cache_file_exists: self._cache_file_exists = True From 30584d93891820d2820a0df79746a5c56829c054 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 15:57:44 +0200 Subject: [PATCH 18/19] Use suppress insteady of try-except --- plugwise_usb/helpers/cache.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 8046777b2..0d5eb5237 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -146,27 +146,23 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None await temp_file.writelines(data_to_write) await temp_file.flush() # Ensure data reaches disk before rename - try: + with suppress(OSError, TypeError, AttributeError): + # If fsync fails due to fileno() issues or other problems, + # continue without it. Flush() provides reasonable durability. loop = get_running_loop() await loop.run_in_executor(None, os_fsync, temp_file.fileno()) - except (OSError, TypeError, AttributeError): - # If fsync fails due to fileno() issues or other problems, - # continue without it. flush() provides reasonable durability. - pass # Atomic rename (overwrites atomically on all platforms) temp_path.replace(cache_file_path) temp_path = None # Successfully renamed if os_name != "nt": # Ensure directory entry is persisted on POSIX - try: + with suppress(OSError, PermissionError): + # Directory fsync may fail on some filesystems + # The atomic replace is still complete await loop.run_in_executor( None, _fsync_parent_dir, cache_file_path.parent ) - except (OSError, PermissionError): - # Directory fsync may fail on some filesystems - # The atomic rename is still complete - pass if not self._cache_file_exists: self._cache_file_exists = True From 825b6762d8b22f3d00fbebe055e918254bcbe39a Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 15 Sep 2025 16:26:54 +0200 Subject: [PATCH 19/19] Remove all POSIX related suggestions --- plugwise_usb/helpers/cache.py | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/plugwise_usb/helpers/cache.py b/plugwise_usb/helpers/cache.py index 0d5eb5237..ef79cf2af 100644 --- a/plugwise_usb/helpers/cache.py +++ b/plugwise_usb/helpers/cache.py @@ -5,15 +5,7 @@ from asyncio import get_running_loop from contextlib import suppress import logging -from os import ( - O_RDONLY as os_O_RDONLY, - close as os_close, - fsync as os_fsync, - getenv as os_getenv, - getpid as os_getpid, - name as os_name, - open as os_open, -) +from os import getenv as os_getenv, getpid as os_getpid, name as os_name from os.path import expanduser as os_path_expand_user, join as os_path_join from pathlib import Path from secrets import token_hex as secrets_token_hex @@ -30,15 +22,6 @@ _LOGGER = logging.getLogger(__name__) -def _fsync_parent_dir(path: Path) -> None: - """Ensure persistence on POSIX.""" - fd = os_open(str(path), os_O_RDONLY) - try: - os_fsync(fd) - finally: - os_close(fd) - - class PlugwiseCache: """Base class to cache plugwise information.""" @@ -144,25 +127,12 @@ async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None newline="\n", ) as temp_file: await temp_file.writelines(data_to_write) + # Ensure buffered data is written await temp_file.flush() - # Ensure data reaches disk before rename - with suppress(OSError, TypeError, AttributeError): - # If fsync fails due to fileno() issues or other problems, - # continue without it. Flush() provides reasonable durability. - loop = get_running_loop() - await loop.run_in_executor(None, os_fsync, temp_file.fileno()) # Atomic rename (overwrites atomically on all platforms) temp_path.replace(cache_file_path) temp_path = None # Successfully renamed - if os_name != "nt": - # Ensure directory entry is persisted on POSIX - with suppress(OSError, PermissionError): - # Directory fsync may fail on some filesystems - # The atomic replace is still complete - await loop.run_in_executor( - None, _fsync_parent_dir, cache_file_path.parent - ) if not self._cache_file_exists: self._cache_file_exists = True