Add option to log to file mount SD Card #299
Conversation
pysquared/sd_card.py
Outdated
| chip_select: Pin, | ||
| baudrate: int = 400000, | ||
| mount_path: str = "/sd", | ||
| mounted: bool = False, |
There was a problem hiding this comment.
A caller probably shouldn't be able to set the mounted state when creating an SD()
pysquared/sd_card.py
Outdated
| mount_path: str = "/sd", | ||
| mounted: bool = False, | ||
| log_size: int = 50000, # 50 kb | ||
| log_rotate: int = 10, |
There was a problem hiding this comment.
Since we're not including log rotation in this PR, let's remove all mention of it.
pysquared/sd_card.py
Outdated
| log_rotate: int = 10, | ||
| ) -> None: | ||
| self.mounted = mounted | ||
| self.log_size = log_size |
pysquared/sd_card.py
Outdated
| else: | ||
| self.mounted = True |
There was a problem hiding this comment.
While this appears to be valid it's also unusual to see a try/else. How about just putting this at the end of try?
pysquared/sd_card.py
Outdated
| if "logs" not in os.listdir("/sd"): | ||
| print("/sd/logs does not exist, creating...") | ||
| os.mkdir("/sd/logs") |
There was a problem hiding this comment.
Since SD is our generic manager for handling SD card mounting, the specific usage of the SD card by the logger should likely live in the logger.
pysquared/sd_card.py
Outdated
| print("error mounting sd card", e) | ||
| else: | ||
| self.mounted = True |
There was a problem hiding this comment.
Instead of using the concept of mounted=True let's raise a hardware initialization error like other managers.
pysquared/logger.py
Outdated
| def __init__( | ||
| self, | ||
| error_counter: Counter, | ||
| sd_card: SDCardManager = None, |
There was a problem hiding this comment.
Yeah, I was wrong about this. Instead of accepting an SDCard here we should accept a path. If a path is set, we should verify it exists during the init if it doesn't raise an error.
There was a problem hiding this comment.
Hey Nate, first I just wanted to say thank you for all of your help so far with this PR! I'm finally about done getting it updated, but was having some trouble understanding the intentions of this recommendation. Because the sd card cannot be initialized before the logger, if the path is ever set on initialization (ex: sd_path = sd/logs) it can never exist, or if it does, the sd card will not be mounted to it which I'm not sure what the resulting behavior would be if we tried writing to that path. Maybe there could be a function within the logger to set the sd_path, verifying it exists when set? Another thing that I wasn't sure of was when verifying if it exists, would we then want to create the new directory and then raise an error, sort of similar to what is implemented now?:
if "logs" not in os.listdir("/sd"):
print("/sd/logs does not exist, creating...")
os.mkdir("/sd/logs")
Not sure if I am just misunderstanding how the path works or just the implementation you are imagining with the comment.
Thanks again for all the help!
…le inside pysquared
|
2 participants
Summary
Created SDCardManager class to initialize and mount sd card, as well as adding functionality to the logger in order to set a log path for the sd card. The log path is set using a setter within the logger and checks that the directory exists before writing.
This PR also creates a new boot.py function that creates set mount points on the root of the file system.
How was this tested
boot_out.txtfirst run:Adafruit CircuitPython 9.2.8 on 2025-05-28; PROVES Kit v4 with rp2040 Board ID:proveskit_rp2040_v4 UID:E4639C3563112E2D boot.py output: Disabled USB drive Remounted root filesystem Mount point /poke created. Enabled USB driveboot_out.txtsubsequent runs:Adafruit CircuitPython 9.2.8 on 2025-05-28; PROVES Kit v4 with rp2040 Board ID:proveskit_rp2040_v4 UID:E4639C3563112E2D boot.py output: Disabled USB drive Remounted root filesystem Mount point /poke already exists. Enabled USB driveDemonstrating logging to file:
