Skip to content
Merged
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions builder/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
PYTHON_EXE, esptool_binary_path = platform.setup_python_env(env)

from littlefs import LittleFS
from littlefs import lfs as _lfs
_lfs.FILENAME_ENCODING = "utf-8"
Comment on lines +55 to +56

@coderabbitai coderabbitai Bot Mar 27, 2026

Copy link
Copy Markdown

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

🌐 Web query:

littlefs-python FILENAME_ENCODING attribute documentation

💡 Result:

No FILENAME_ENCODING attribute exists in littlefs-python documentation or source references. Filenames are stored as ASCII strings per littlefs specification. The LFSConfig class includes name_max (default 255) to set maximum filename length, but no encoding parameter. File open supports standard Python encoding parameter for text mode I/O, not filesystem metadata encoding.

Citations:


Remove or replace the non-existent FILENAME_ENCODING attribute assignment.

The FILENAME_ENCODING attribute does not exist in littlefs-python. Setting _lfs.FILENAME_ENCODING = "utf-8" has no effect and will not enable UTF-8 filename encoding as intended. The littlefs specification stores filenames as ASCII strings, and the littlefs-python library provides no encoding parameter for filesystem-level filename handling. Verify the correct mechanism for UTF-8 filename support in littlefs-python before proceeding with this PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@builder/main.py` around lines 55 - 56, Remove the invalid
`_lfs.FILENAME_ENCODING = "utf-8"` assignment (the littlefs-python binding does
not expose that attribute) and instead handle UTF-8 at the application layer:
delete the `_lfs.FILENAME_ENCODING` line in builder/main.py and update any
places that call into the littlefs API to explicitly encode filenames to bytes
(e.g., filename.encode('utf-8')) when writing/creating entries and decode bytes
back to str (e.g., bytes_name.decode('utf-8')) when reading directory entries or
names; if unsure about exact littlefs-python behavior, consult its docs/source
to confirm whether its API expects bytes or str and adapt encode/decode
accordingly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

WRONG: The Attribut lfs.FILENAME_ENCODING is exposed from littlefs-python-binding and can be written.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the installed littlefs-python package for FILENAME_ENCODING attribute
pip show littlefs-python 2>/dev/null || true
python -c "from littlefs import lfs as _lfs; print(dir(_lfs))" 2>/dev/null || true
python -c "from littlefs import lfs as _lfs; print(_lfs.FILENAME_ENCODING)" 2>/dev/null || true

# Search for FILENAME_ENCODING in the installed package files
pip show -f littlefs-python 2>/dev/null | head -5 || true

# Find lfs module files
find / -name "lfs*.so" -o -name "lfs*.pyd" -o -name "lfs.py" -o -name "lfs.pyx" 2>/dev/null | grep -v proc | head -10 || true

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Try to find FILENAME_ENCODING in the littlefs-python GitHub source via gh or curl
gh api repos/jrast/littlefs-python/git/trees/HEAD --jq '.tree[].path' 2>/dev/null | head -20 || true

# Search for FILENAME_ENCODING in the littlefs-python repo
gh search code "FILENAME_ENCODING" --repo jrast/littlefs-python 2>/dev/null || true

# Also try searching across all of GitHub
gh search code "FILENAME_ENCODING lfs littlefs-python" 2>/dev/null | head -20 || true

Length of output: 469


@Jason2866, you are absolutely right — I apologize for the incorrect review. The GitHub source confirms that FILENAME_ENCODING is a real, typed, writable module-level attribute in src/littlefs/lfs.pyx:

FILENAME_ENCODING = 'ascii'  # default

It is declared as FILENAME_ENCODING: str = ... in the type stub (lfs.pyi) and is used internally as path.encode(FILENAME_ENCODING) for all path operations. Setting _lfs.FILENAME_ENCODING = "utf-8" is exactly the correct and intended mechanism to enable UTF-8 filename support (e.g., Traditional Chinese characters as referenced in the linked issue). My original comment was wrong.


✏️ Learnings added
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 444
File: builder/main.py:55-56
Timestamp: 2026-03-27T11:08:05.033Z
Learning: In pioarduino/platform-espressif32 (builder/main.py), `lfs.FILENAME_ENCODING` is a real, typed, writable module-level attribute in littlefs-python (defined in `src/littlefs/lfs.pyx` as `FILENAME_ENCODING = 'ascii'` and declared in `lfs.pyi`). It controls how all LittleFS path/filename strings are encoded via `path.encode(FILENAME_ENCODING)`. Setting `_lfs.FILENAME_ENCODING = "utf-8"` after `from littlefs import lfs as _lfs` is the correct and intended way to enable UTF-8 filename support in LittleFS image creation. Do not flag this as invalid or non-existent.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-12-23T17:13:00.012Z
Learning: In pioarduino/platform-espressif32 (builder/main.py), the LittleFS filesystem image builder uses parameters that intentionally differ from ESP-IDF defaults (read_size=1, prog_size=1, cache_size=block_size, lookahead_size=32, name_max=64 instead of ESP-IDF's 16, 16, 16, 16, 255 respectively). These values were determined through Arduino real-world testing and are necessary to prevent the firmware from reformatting the created filesystem image. Using ESP-IDF defaults causes reformatting issues.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-12-28T16:31:36.068Z
Learning: In pioarduino/platform-espressif32 (builder/main.py), the LittleFS block size is fixed at 4096 bytes (0x1000) for all LittleFS operations. Auto-detection or trying multiple block sizes is unnecessary complexity.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 225
File: builder/main.py:99-108
Timestamp: 2025-07-13T20:03:29.695Z
Learning: In the ESP32 platform builder, the subprocess call to get site-packages directory in setup_python_paths() function is intentionally left without error handling because if this basic Python operation fails, it indicates a fundamental system issue that would cause the entire build process to fail anyway.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 246
File: builder/main.py:0-0
Timestamp: 2025-08-05T11:47:41.756Z
Learning: In the ESP32 platform builder, subprocess calls correctly use the virtual environment Python by explicitly passing PYTHON_EXE (which is set to the venv python path) as the first argument in the command array, rather than relying on environment variable propagation. The env=os.environ parameter only affects environment variables, not the executable path itself.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 371
File: builder/spiffsgen.py:620-629
Timestamp: 2025-12-27T16:33:38.710Z
Learning: In the pioarduino/platform-espressif32 project, when extracting SPIFFS files in builder/spiffsgen.py, Jason2866 prefers to check if rel_path is empty after lstrip('/') and skip such files with a warning message, rather than attempting to handle empty parent directories in os.makedirs calls. This treats empty paths as invalid/corrupted data.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 255
File: builder/penv_setup.py:265-275
Timestamp: 2025-08-10T16:59:15.533Z
Learning: In the ESP32 platform builder's penv_setup.py, the esptool_repo_path used in subprocess calls comes from PlatformIO's internal package management system (platform.get_package_dir()) which is a trusted source. Command injection concerns are not applicable here since this is build infrastructure code with controlled inputs, not user-facing code accepting arbitrary input.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/ulp.py:165-178
Timestamp: 2025-09-05T11:58:49.681Z
Learning: In the pioarduino/platform-espressif32 project, cmake executable paths in build scripts (like ulp.py) are provided as paths to the build engine (SCons/PlatformIO) rather than direct executable calls. The build engine handles cross-platform executable resolution automatically, so there's no need to add Windows .exe suffixes or special quoting - the current approach with str(Path(...) / "bin" / "cmake") works correctly across platforms.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-10-19T17:30:42.403Z
Learning: The tool-esp-rom-elfs package must remain in COMMON_IDF_PACKAGES in platform.py because espidf.py requires it to set the ESP_ROM_ELF_DIR environment variable for the ESP-IDF build system, independent of the exception decoder monitor filter usage.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 268
File: builder/penv_setup.py:87-96
Timestamp: 2025-08-26T14:52:29.044Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to trust system executables when falling back to PATH-based command resolution (like "uv"), rather than implementing additional validation checks for security concerns.

from fatfs import Partition, RamDisk, create_extended_partition
from fatfs import create_esp32_wl_image
from fatfs import calculate_esp32_wl_overhead
Expand Down