ojars/MeshCore
Watch
1
0
Fork
You've already forked MeshCore
 0
Code Pull requests Projects Releases Packages Activity

fix(ci): restore D1L build + make logger portable #51

Merged
okapteinis merged 1 commit from fix/ci-d1l-followups into nightly 2025-12-28 16:54:55 +02:00
Conversation 2 Commits 1 Files changed 5 +147 -31
okapteinis commented 2025-12-28 16:53:01 +02:00 (Migrated from github.com)
Copy link

Fixes:

  • Add missing pinToInterrupt() declaration in CustomRadioLibHal.h Resolves CI error: 'no declaration matches uint32_t CustomRadioLibHal::pinToInterrupt(uint32_t)'

Gemini follow-ups from PR #50:

  • Make tools/d1l_logger.py portable:

    • Remove hardcoded LOG_DIR and SERIAL_PORT
    • Add argparse for --port, --baud, --log-dir options
    • Add auto-detection for macOS/Linux serial ports
    • Use pathlib.Path for cross-platform paths
    • Create log directory if missing
    • Update command examples in CLAUDE.md and D1L_GUIDE.md

  • Fix docs/hardware/D1L_GUIDE.md snippet mismatch:

    • Update RecursiveSemaphoreLockGuard code to match freertos_util.h
    • Add missing '5s timeout' text and 'Possible deadlock' message
    • Add #ifdef ARDUINO guard

Gemini follow-ups from PR #49:

  • Replace Serial.printf/println with log_* macros in target.cpp:
    • Use log_e() for fatal/error messages
    • Use log_w() for warnings
    • Use log_i() for informational messages
    • Improves logging consistency across codebase

Verified:

  • Local build succeeds: pio run -e SenseCapIndicator-D1L_repeater
  • All fixes applied as requested

Description

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Hardware support addition
  • Performance improvement

Related Issues

Testing Performed

  • Builds successfully on affected platforms
  • Tested on physical hardware
  • Verified no new compiler warnings
  • Checked memory usage
  • Example applications still work
  • Documentation updated

Hardware Tested

  • Device:
  • Firmware variant:
  • Test results:

Code Quality Checklist

  • Follows existing code style
  • No dynamic memory allocation in inappropriate places
  • No reformatting of existing code
  • Added comments for non-obvious logic
  • Updated documentation
  • CHANGELOG.md updated if applicable

Breaking Changes

Screenshots/Logs

Additional Notes

Fixes: - Add missing pinToInterrupt() declaration in CustomRadioLibHal.h Resolves CI error: 'no declaration matches uint32_t CustomRadioLibHal::pinToInterrupt(uint32_t)' Gemini follow-ups from PR #50: - Make tools/d1l_logger.py portable: * Remove hardcoded LOG_DIR and SERIAL_PORT * Add argparse for --port, --baud, --log-dir options * Add auto-detection for macOS/Linux serial ports * Use pathlib.Path for cross-platform paths * Create log directory if missing * Update command examples in CLAUDE.md and D1L_GUIDE.md - Fix docs/hardware/D1L_GUIDE.md snippet mismatch: * Update RecursiveSemaphoreLockGuard code to match freertos_util.h * Add missing '5s timeout' text and 'Possible deadlock' message * Add #ifdef ARDUINO guard Gemini follow-ups from PR #49: - Replace Serial.printf/println with log_* macros in target.cpp: * Use log_e() for fatal/error messages * Use log_w() for warnings * Use log_i() for informational messages * Improves logging consistency across codebase Verified: - Local build succeeds: pio run -e SenseCapIndicator-D1L_repeater - All fixes applied as requested ## Description <!-- Provide a clear description of what this PR does --> ## Type of Change <!-- Mark relevant items with [x] --> - [ ] Bug fix (non-breaking change fixing an issue) - [ ] New feature (non-breaking change adding functionality) - [ ] Breaking change (fix or feature causing existing functionality to change) - [ ] Documentation update - [ ] Hardware support addition - [ ] Performance improvement ## Related Issues <!-- Link related issues: Fixes #123, Relates to #456 --> ## Testing Performed <!-- Describe testing done --> - [ ] Builds successfully on affected platforms - [ ] Tested on physical hardware - [ ] Verified no new compiler warnings - [ ] Checked memory usage - [ ] Example applications still work - [ ] Documentation updated ## Hardware Tested <!-- List hardware tested, or write "N/A" if documentation-only --> - Device: - Firmware variant: - Test results: ## Code Quality Checklist - [ ] Follows existing code style - [ ] No dynamic memory allocation in inappropriate places - [ ] No reformatting of existing code - [ ] Added comments for non-obvious logic - [ ] Updated documentation - [ ] CHANGELOG.md updated if applicable ## Breaking Changes <!-- Describe any breaking changes and migration path, or write "None" --> ## Screenshots/Logs <!-- Add screenshots or serial logs if applicable --> ## Additional Notes <!-- Any additional information for reviewers -->
gemini-code-assist[bot] commented 2025-12-28 16:53:19 +02:00 (Migrated from github.com)
Copy link

Summary of Changes

Hello @okapteinis, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on improving the robustness and usability of the D1L build and its associated tooling. It addresses a critical CI build failure by correctly implementing a hardware abstraction layer function, enhances the D1L serial logger script for greater portability and ease of use across different operating systems, and refines the embedded device's logging mechanism for better consistency. Additionally, it updates relevant documentation to reflect these changes and prevent future discrepancies.

Highlights

  • CI Build Fix: Resolved a CI error by adding a missing pinToInterrupt() declaration in CustomRadioLibHal.h to correctly handle virtual pins, ensuring the D1L build succeeds.
  • Portable D1L Logger: Enhanced tools/d1l_logger.py for cross-platform compatibility by removing hardcoded paths/ports, introducing argparse for flexible options, enabling auto-detection of serial ports on macOS/Linux, and utilizing pathlib.Path for robust path handling.
  • Documentation Updates: Synchronized CLAUDE.md and D1L_GUIDE.md with the new logger usage examples and corrected a RecursiveSemaphoreLockGuard code snippet in D1L_GUIDE.md to include a 5-second timeout, potential deadlock messages, and an #ifdef ARDUINO guard.
  • Consistent Logging: Replaced Serial.printf and Serial.println calls with log_e, log_w, and log_i macros in target.cpp for improved logging consistency and severity-based output.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩︎

## Summary of Changes Hello @okapteinis, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on improving the robustness and usability of the D1L build and its associated tooling. It addresses a critical CI build failure by correctly implementing a hardware abstraction layer function, enhances the D1L serial logger script for greater portability and ease of use across different operating systems, and refines the embedded device's logging mechanism for better consistency. Additionally, it updates relevant documentation to reflect these changes and prevent future discrepancies. ### Highlights * **CI Build Fix**: Resolved a CI error by adding a missing `pinToInterrupt()` declaration in `CustomRadioLibHal.h` to correctly handle virtual pins, ensuring the D1L build succeeds. * **Portable D1L Logger**: Enhanced `tools/d1l_logger.py` for cross-platform compatibility by removing hardcoded paths/ports, introducing `argparse` for flexible options, enabling auto-detection of serial ports on macOS/Linux, and utilizing `pathlib.Path` for robust path handling. * **Documentation Updates**: Synchronized `CLAUDE.md` and `D1L_GUIDE.md` with the new logger usage examples and corrected a `RecursiveSemaphoreLockGuard` code snippet in `D1L_GUIDE.md` to include a 5-second timeout, potential deadlock messages, and an `#ifdef ARDUINO` guard. * **Consistent Logging**: Replaced `Serial.printf` and `Serial.println` calls with `log_e`, `log_w`, and `log_i` macros in `target.cpp` for improved logging consistency and severity-based output. <details> <summary><b>Using Gemini Code Assist</b></summary> <br> The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips. <b>Invoking Gemini</b> You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page. Feature | Command | Description --- | --- | --- Code Review | `/gemini review` | Performs a code review for the current pull request in its current state. Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state. Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. Help | `/gemini help` | Displays a list of available commands. <b>Customization</b> To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). <b>Limitations & Feedback</b> Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG). <b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b> </details> [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
gemini-code-assist[bot] (Migrated from github.com) reviewed 2025-12-28 16:54:38 +02:00
gemini-code-assist[bot] (Migrated from github.com) left a comment
Copy link

Code Review

This pull request delivers a good set of fixes and improvements. The logger script tools/d1l_logger.py is now much more portable and user-friendly thanks to the removal of hardcoded paths and the addition of command-line arguments. The accompanying documentation updates are clear and helpful. The fix for the missing pinToInterrupt declaration and the standardization of logging macros in target.cpp improve code quality and consistency. I've added a couple of suggestions for the logger script to further enhance its robustness and cross-platform compatibility.

## Code Review This pull request delivers a good set of fixes and improvements. The logger script `tools/d1l_logger.py` is now much more portable and user-friendly thanks to the removal of hardcoded paths and the addition of command-line arguments. The accompanying documentation updates are clear and helpful. The fix for the missing `pinToInterrupt` declaration and the standardization of logging macros in `target.cpp` improve code quality and consistency. I've added a couple of suggestions for the logger script to further enhance its robustness and cross-platform compatibility.
tools/d1l_logger.py
@ -15,2 +39,4 @@
return None
def main():
parser = argparse.ArgumentParser(
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-28 16:54:38 +02:00
Copy link

medium

The order of results from glob.glob() is not guaranteed. If multiple serial devices match a pattern, the script might connect to a different one each time it's run, which can be confusing. Sorting the results provides deterministic behavior by always picking the first port alphabetically.

        ports = sorted(glob.glob(pattern))
        if ports:
            return ports[0]
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The order of results from `glob.glob()` is not guaranteed. If multiple serial devices match a pattern, the script might connect to a different one each time it's run, which can be confusing. Sorting the results provides deterministic behavior by always picking the first port alphabetically. ```suggestion ports = sorted(glob.glob(pattern)) if ports: return ports[0] ```
tools/d1l_logger.py
@ -16,1 +41,4 @@
def main():
parser = argparse.ArgumentParser(
description='D1L Serial Logger - Portable serial monitor with event logging',
formatter_class=argparse.RawDescriptionHelpFormatter,
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-28 16:54:38 +02:00
Copy link

medium

While using glob works for macOS and Linux, a more portable and robust solution would be to use serial.tools.list_ports.comports(). This function is part of the pyserial library, works cross-platform (including Windows), and allows for more reliable detection based on USB Vendor ID (VID) and Product ID (PID). This would make the script truly portable.

Here's a potential refactoring of find_serial_port:

import serial.tools.list_ports

def find_serial_port():
    """
    Try to auto-detect D1L serial port on any platform.
    Returns None if not found.
    """
    # The SenseCAP Indicator D1L uses a CH9102 USB-to-serial chip (VID:1A86).
    # We can look for this specific vendor.
    D1L_VID = 0x1A86

    ports = serial.tools.list_ports.comports()
    
    # First, try to find a device with the specific VID.
    for port in ports:
        if port.vid == D1L_VID:
            print(f"Found potential D1L device on {port.device}")
            return port.device

    # If not found, fall back to common serial port names.
    for port in ports:
        if "usbserial" in port.device or "ttyUSB" in port.device or "ttyACM" in port.device:
             print(f"Found generic serial device on {port.device}")
             return port.device

    return None
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) While using `glob` works for macOS and Linux, a more portable and robust solution would be to use `serial.tools.list_ports.comports()`. This function is part of the `pyserial` library, works cross-platform (including Windows), and allows for more reliable detection based on USB Vendor ID (VID) and Product ID (PID). This would make the script truly portable. Here's a potential refactoring of `find_serial_port`: ```python import serial.tools.list_ports def find_serial_port(): """ Try to auto-detect D1L serial port on any platform. Returns None if not found. """ # The SenseCAP Indicator D1L uses a CH9102 USB-to-serial chip (VID:1A86). # We can look for this specific vendor. D1L_VID = 0x1A86 ports = serial.tools.list_ports.comports() # First, try to find a device with the specific VID. for port in ports: if port.vid == D1L_VID: print(f"Found potential D1L device on {port.device}") return port.device # If not found, fall back to common serial port names. for port in ports: if "usbserial" in port.device or "ttyUSB" in port.device or "ttyACM" in port.device: print(f"Found generic serial device on {port.device}") return port.device return None ```
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
ojars/MeshCore!51
No description provided.