-
Notifications
You must be signed in to change notification settings - Fork 23
Feedback on SD Card Logging #305
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
Changes from all commits
3b98a1e
ec2583e
4614fff
f0cb400
f600b4a
62f5a68
c984065
1636f84
8f35601
5ab4ef6
04b55a3
358960a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| """Mock for the CircuitPython storage module. | ||
|
|
||
| This module provides a mock implementation of the CircuitPython storage module | ||
| for testing purposes. It allows for simulating the behavior of the storage | ||
| module without the need for actual hardware. | ||
| """ | ||
|
|
||
|
|
||
| def disable_usb_drive() -> None: | ||
| """A mock function to disable the USB drive.""" | ||
| pass | ||
|
|
||
|
|
||
| def enable_usb_drive() -> None: | ||
| """A mock function to enable the USB drive.""" | ||
| pass | ||
|
|
||
|
|
||
| def remount(path: str, readonly: bool) -> None: | ||
| """A mock function to remount the filesystem. | ||
|
|
||
| Args: | ||
| path: The path to remount. | ||
| readonly: Whether to mount as read-only. | ||
| """ | ||
| pass |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| """ | ||
| This module provides utilities that can run during the boot process by adding them to boot.py. | ||
| """ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| """File includes utilities for managing the filesystem during the boot process.""" | ||
|
|
||
| import os | ||
| import time | ||
|
|
||
| try: | ||
| import storage | ||
| except ImportError: | ||
| import mocks.circuitpython.storage as storage | ||
|
|
||
|
|
||
| def mkdir( | ||
| path: str, | ||
| storage_action_delay: float = 0.02, | ||
| ) -> None: | ||
| """ | ||
| Create directories on internal storage during boot. | ||
|
|
||
| In CircuitPython the internal storage is not writable by default. In order to mount | ||
| any external storage (such as an SD Card) the drive must be remounted in read/write mode. | ||
| This function handles the necessary steps to safely create a directory on the internal | ||
| storage during boot. | ||
|
|
||
| Args: | ||
| mount_point: Path to mount point | ||
| storage_action_delay: Delay after storage actions to ensure stability | ||
|
|
||
| Usage: | ||
| ```python | ||
| from pysquared.boot.filesystem import mkdir | ||
| mkdir("/sd") | ||
| ``` | ||
| """ | ||
| try: | ||
| storage.disable_usb_drive() | ||
| time.sleep(storage_action_delay) | ||
| print("Disabled USB drive") | ||
|
|
||
| storage.remount("/", False) | ||
| time.sleep(storage_action_delay) | ||
| print("Remounted root filesystem") | ||
|
|
||
| try: | ||
| os.mkdir(path) | ||
| print(f"Mount point {path} created.") | ||
| except OSError: | ||
| print(f"Mount point {path} already exists.") | ||
|
|
||
| finally: | ||
| storage.enable_usb_drive() | ||
| time.sleep(storage_action_delay) | ||
| print("Enabled USB drive") |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| """ | ||
| This module provides an interface for controlling SD cards. | ||
| """ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,8 +18,6 @@ | |
| import traceback | ||
| from collections import OrderedDict | ||
|
|
||
| import adafruit_pathlib | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I previously suggested using
So ultimately, I removed it. |
||
|
|
||
| from .nvm.counter import Counter | ||
|
|
||
|
|
||
|
|
@@ -79,31 +77,25 @@ class LogLevel: | |
| class Logger: | ||
| """Handles logging messages with different severity levels.""" | ||
|
|
||
| _log_dir: str | None = None | ||
|
|
||
| def __init__( | ||
| self, | ||
| error_counter: Counter, | ||
| sd_path: adafruit_pathlib.Path = None, | ||
| # sd_card: SDCardManager = None, | ||
| log_level: int = LogLevel.NOTSET, | ||
| colorized: bool = False, | ||
| ) -> None: | ||
| """ | ||
| Initializes the Logger instance. | ||
|
|
||
| Args: | ||
| error_counter (Counter): Counter for error occurrences. | ||
| log_level (int): Initial log level. | ||
| colorized (bool): Whether to colorize output. | ||
| error_counter: Counter for error occurrences. | ||
| log_level: Initial log level. | ||
| colorized: Whether to colorize output. | ||
| """ | ||
| self._error_counter: Counter = error_counter | ||
| self.sd_path: adafruit_pathlib.Path = sd_path | ||
| self._log_level: int = log_level | ||
| self.colorized: bool = colorized | ||
|
|
||
| try: | ||
| self.sd_path = self.sd_path / "logs" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to have the logger be opinionated about the directory location so now we just accept whatever path is provided to the logger. |
||
| except TypeError as e: | ||
| print(f"path not set: {e}") | ||
| self._colorized: bool = colorized | ||
|
|
||
| def _can_print_this_level(self, level_value: int) -> bool: | ||
| """ | ||
|
|
@@ -162,30 +154,15 @@ def _log(self, level: str, level_value: int, message: str, **kwargs) -> None: | |
|
|
||
| json_order.update(kwargs) | ||
|
|
||
| try: | ||
| json_output = json.dumps(json_order) | ||
| except TypeError as e: | ||
| json_output = json.dumps( | ||
| OrderedDict( | ||
| [ | ||
| ("time", asctime), | ||
| ("level", "ERROR"), | ||
| ("msg", f"Failed to serialize log message: {e}"), | ||
| ] | ||
| ), | ||
| ) | ||
|
Comment on lines
-165
to
-176
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This became no longer necessary after #228 but was left in. It has no tests and is no longer required so I removed it. |
||
| json_output = json.dumps(json_order) | ||
|
|
||
| if self._can_print_this_level(level_value): | ||
| # Write to sd card if mounted | ||
| if self.path: | ||
| if "logs" not in self.sd_path.iterdir(): | ||
| print("/sd/logs does not exist, creating...") | ||
| os.mkdir("/sd/logs") | ||
|
|
||
| with open("/sd/logs/activity.log", "a") as f: | ||
| if self._log_dir is not None: | ||
| file = self._log_dir + os.sep + "activity.log" | ||
| with open(file, "a") as f: | ||
| f.write(json_output + "\n") | ||
|
|
||
| if self.colorized: | ||
| if self._colorized: | ||
| json_output = json_output.replace( | ||
| f'"level": "{level}"', f'"level": "{LogColors[level]}"' | ||
| ) | ||
|
|
@@ -256,3 +233,26 @@ def get_error_count(self) -> int: | |
| int: The number of errors logged. | ||
| """ | ||
| return self._error_counter.get() | ||
|
|
||
| def set_log_dir(self, log_dir: str) -> None: | ||
| """ | ||
| Sets the log directory for file logging. | ||
|
|
||
| Args: | ||
| log_dir (str): Directory to save log files. | ||
|
|
||
| Raises: | ||
| ValueError: If the provided path is not a valid directory. | ||
| """ | ||
| try: | ||
| # Octal number 0o040000 is the stat mode indicating the file being stat'd is a directory | ||
| directory_mode: int = 0o040000 | ||
| st_mode = os.stat(log_dir)[0] | ||
| if st_mode != directory_mode: | ||
| raise ValueError( | ||
| f"Logging path must be a directory, received {st_mode}." | ||
| ) | ||
| except OSError as e: | ||
| raise ValueError("Invalid logging path.") from e | ||
|
|
||
| self._log_dir = log_dir | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this PR: switching this order is possible now because of #304. With this order change, the IDE shows more accurate hints.