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

fix(d1l): workaround RadioLib IRQ pin bug and apply Gemini cleanup #49

Merged
okapteinis merged 1 commit from fix/d1l-radio-and-cleanup into nightly 2025-12-27 21:29:22 +02:00
Conversation 2 Commits 1 Files changed 4 +90 -97
okapteinis commented 2025-12-27 21:29:02 +02:00 (Migrated from github.com)
Copy link

This commit addresses two issues discovered during D1L INT pin optimization:

  1. CRITICAL FIX: RadioLib Module IRQ pin reset bug

    • RadioLib's radio.begin() resets Module IRQ pin to -1
    • Added verification and restoration of correct pin (103) in target.cpp
    • Prevents "attachInterrupt real pin -1" error that blocks transmission

  2. Gemini Code Review Cleanup:

    • Refactored pollVirtualInterruptsInternal() to use readAllInputs()
    • Eliminates code duplication between polling and INT modes
    • Updated documentation to reflect actual do/while loop implementation
    • Updated documentation to show efficient read16() port-based reads

Changes:

  • variants/sensecap_indicator_d1l/target.cpp: Add Module IRQ pin workaround
  • variants/sensecap_indicator_d1l/hal/CustomRadioLibHal.cpp: Refactor polling
  • docs/hardware/d1l_int_pin_optimization.md: Update INT handler code
  • docs/hardware/d1l_int_pin_implementation_summary.md: Update readAllInputs

Testing:

  • Build verified: pio run -e SenseCapIndicator-D1L_repeater (SUCCESS)
  • No compilation errors or warnings
  • Ready for hardware testing

🤖 Generated with Claude Code

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

This commit addresses two issues discovered during D1L INT pin optimization: 1. CRITICAL FIX: RadioLib Module IRQ pin reset bug - RadioLib's radio.begin() resets Module IRQ pin to -1 - Added verification and restoration of correct pin (103) in target.cpp - Prevents "attachInterrupt real pin -1" error that blocks transmission 2. Gemini Code Review Cleanup: - Refactored pollVirtualInterruptsInternal() to use readAllInputs() - Eliminates code duplication between polling and INT modes - Updated documentation to reflect actual do/while loop implementation - Updated documentation to show efficient read16() port-based reads Changes: - variants/sensecap_indicator_d1l/target.cpp: Add Module IRQ pin workaround - variants/sensecap_indicator_d1l/hal/CustomRadioLibHal.cpp: Refactor polling - docs/hardware/d1l_int_pin_optimization.md: Update INT handler code - docs/hardware/d1l_int_pin_implementation_summary.md: Update readAllInputs Testing: - Build verified: pio run -e SenseCapIndicator-D1L_repeater (SUCCESS) - No compilation errors or warnings - Ready for hardware testing 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## 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-27 21:29: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 enhances the stability and efficiency of the D1L device's radio communication and interrupt handling. It resolves a critical bug affecting the RadioLib module's interrupt pin initialization and streamlines I2C operations for GPIO expander reads, leading to more reliable and performant system behavior.

Highlights

  • RadioLib IRQ Pin Bug Fix: Implemented a critical workaround in target.cpp to address a RadioLib bug where radio.begin() incorrectly resets the Module IRQ pin. The fix verifies and restores the correct pin (103) to prevent "attachInterrupt real pin -1" errors and ensure proper radio transmission.
  • I2C Read Efficiency and Interrupt Handling Refinement: Refactored pollVirtualInterruptsInternal() in CustomRadioLibHal.cpp to leverage readAllInputs(), eliminating code duplication and significantly reducing I2C transactions (from 16 to 2) for reading all 16 TCA9535 inputs. The interrupt handler documentation and implementation were updated to use a do/while loop for robust level-triggered interrupt handling, preventing missed events.
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 enhances the stability and efficiency of the D1L device's radio communication and interrupt handling. It resolves a critical bug affecting the RadioLib module's interrupt pin initialization and streamlines I2C operations for GPIO expander reads, leading to more reliable and performant system behavior. ### Highlights * **RadioLib IRQ Pin Bug Fix**: Implemented a critical workaround in `target.cpp` to address a RadioLib bug where `radio.begin()` incorrectly resets the Module IRQ pin. The fix verifies and restores the correct pin (103) to prevent "attachInterrupt real pin -1" errors and ensure proper radio transmission. * **I2C Read Efficiency and Interrupt Handling Refinement**: Refactored `pollVirtualInterruptsInternal()` in `CustomRadioLibHal.cpp` to leverage `readAllInputs()`, eliminating code duplication and significantly reducing I2C transactions (from 16 to 2) for reading all 16 TCA9535 inputs. The interrupt handler documentation and implementation were updated to use a `do/while` loop for robust level-triggered interrupt handling, preventing missed events. <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-27 21:30:52 +02:00
gemini-code-assist[bot] (Migrated from github.com) left a comment
Copy link

Code Review

This pull request introduces a critical workaround for a RadioLib bug that resets the IRQ pin, and also includes a significant performance improvement by refactoring the virtual interrupt polling logic. The changes are well-documented and the refactoring correctly reduces I2C bus traffic. My feedback focuses on improving logging consistency in one of the changed files to align with the project's existing practices.

## Code Review This pull request introduces a critical workaround for a RadioLib bug that resets the IRQ pin, and also includes a significant performance improvement by refactoring the virtual interrupt polling logic. The changes are well-documented and the refactoring correctly reduces I2C bus traffic. My feedback focuses on improving logging consistency in one of the changed files to align with the project's existing practices.
variants/sensecap_indicator_d1l/target.cpp
@ -445,0 +456,4 @@
Serial.printf("DEBUG: Module IRQ pin corrected to %d\n", radio->mod->irqPin);
} else {
Serial.println("DEBUG: Module IRQ pin is correct");
}
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-27 21:30:52 +02:00
Copy link

medium

The use of Serial.printf and Serial.println for debugging and warnings is inconsistent with the existing use of the ESP-IDF logging framework (log_w, log_e, etc.) in this file. Using the logging framework provides benefits like log levels, filtering, and performance.

I suggest replacing the Serial calls with the appropriate log_* macros. This will make the logging consistent and controllable via CORE_DEBUG_LEVEL.

  log_d("Checking Module IRQ pin after radio.begin()...");
  log_d("Module IRQ pin = %d (expected: %d)",
                radio->mod->irqPin, TCA9535_GPIO::LORA_DIO1);

  if (radio->mod->irqPin != TCA9535_GPIO::LORA_DIO1) {
    log_w("Module IRQ pin was reset! Fixing...");
    radio->mod->irqPin = TCA9535_GPIO::LORA_DIO1;
    log_d("Module IRQ pin corrected to %d", radio->mod->irqPin);
  } else {
    log_d("Module IRQ pin is correct");
  }
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The use of `Serial.printf` and `Serial.println` for debugging and warnings is inconsistent with the existing use of the ESP-IDF logging framework (`log_w`, `log_e`, etc.) in this file. Using the logging framework provides benefits like log levels, filtering, and performance. I suggest replacing the `Serial` calls with the appropriate `log_*` macros. This will make the logging consistent and controllable via `CORE_DEBUG_LEVEL`. ```suggestion log_d("Checking Module IRQ pin after radio.begin()..."); log_d("Module IRQ pin = %d (expected: %d)", radio->mod->irqPin, TCA9535_GPIO::LORA_DIO1); if (radio->mod->irqPin != TCA9535_GPIO::LORA_DIO1) { log_w("Module IRQ pin was reset! Fixing..."); radio->mod->irqPin = TCA9535_GPIO::LORA_DIO1; log_d("Module IRQ pin corrected to %d", radio->mod->irqPin); } else { log_d("Module IRQ pin is correct"); } ```
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!49
No description provided.