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

fix(d1l): implement proper RADIO_DRIVER_AVAILABLE guards for target f… #41

Merged
okapteinis merged 1 commit from feature/d1l-radio-driver-proper-guards into nightly 2025-12-26 23:53:10 +02:00
Conversation 2 Commits 1 Files changed 3 +16 -3
okapteinis commented 2025-12-26 23:41:11 +02:00 (Migrated from github.com)
Copy link

…iles

This fix properly implements Option 1 (unguarded globals, guarded initialization) using the RADIO_DRIVER_AVAILABLE flag that is now standard in the nightly branch.

Changes to target.h:

  • Add LORA_TX_POWER=20 fallback for builds that don't define it
  • Use CustomSX1262Wrapper directly instead of WRAPPER_CLASS macro to ensure radio_driver declaration is always valid regardless of build config

Changes to target.cpp:

  • Add LORA_TX_POWER=20 fallback for compatibility
  • Wrap radio initialization logic (HAL, SPI, radio setup) with #if RADIO_DRIVER_AVAILABLE guard
  • Keep non-radio initialization (SPIFFS, clocks) unguarded
  • Global radio variable definitions remain unguarded (always visible to linker)

Changes to platformio.ini:

  • Add RADIO_DRIVER_AVAILABLE=1 to repeater environment to enable radio init

Result:
✓ Repeater builds get full radio functionality
✓ Companion target.cpp compiles successfully without radio hardware ✓ Global radio_driver variable always available to satisfy linker ✓ Radio initialization only runs when RADIO_DRIVER_AVAILABLE is defined

Note: Companion example code (MyMesh.cpp, UITask.cpp) and repeater MyMesh.cpp have separate pre-existing issues that should be addressed in dedicated PRs.

🤖 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

…iles This fix properly implements Option 1 (unguarded globals, guarded initialization) using the RADIO_DRIVER_AVAILABLE flag that is now standard in the nightly branch. Changes to target.h: - Add LORA_TX_POWER=20 fallback for builds that don't define it - Use CustomSX1262Wrapper directly instead of WRAPPER_CLASS macro to ensure radio_driver declaration is always valid regardless of build config Changes to target.cpp: - Add LORA_TX_POWER=20 fallback for compatibility - Wrap radio initialization logic (HAL, SPI, radio setup) with #if RADIO_DRIVER_AVAILABLE guard - Keep non-radio initialization (SPIFFS, clocks) unguarded - Global radio variable definitions remain unguarded (always visible to linker) Changes to platformio.ini: - Add RADIO_DRIVER_AVAILABLE=1 to repeater environment to enable radio init Result: ✓ Repeater builds get full radio functionality ✓ Companion target.cpp compiles successfully without radio hardware ✓ Global radio_driver variable always available to satisfy linker ✓ Radio initialization only runs when RADIO_DRIVER_AVAILABLE is defined Note: Companion example code (MyMesh.cpp, UITask.cpp) and repeater MyMesh.cpp have separate pre-existing issues that should be addressed in dedicated PRs. 🤖 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-26 23:41:25 +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 refines the radio driver integration for the d1l target, specifically addressing how radio functionality is enabled and initialized. It introduces a standardized RADIO_DRIVER_AVAILABLE flag to guard radio-specific code, ensuring that builds without radio hardware compile successfully while full radio functionality is available for enabled configurations. This change also standardizes the LORA_TX_POWER definition and simplifies the global radio variable declaration.

Highlights

  • Conditional Radio Initialization: Implemented proper RADIO_DRIVER_AVAILABLE guards to conditionally compile and initialize radio-related code, ensuring radio functionality only when explicitly enabled.
  • LORA_TX_POWER Fallback: Added a fallback definition for LORA_TX_POWER to 20 in target.h and target.cpp for builds that do not explicitly define this value.
  • Global Radio Variable Declaration: Modified target.h to directly declare radio_driver as CustomSX1262Wrapper, removing the WRAPPER_CLASS macro and its conditional guard to ensure it's always visible to the linker.
  • Repeater Build Configuration: Enabled radio initialization for the repeater environment by adding RADIO_DRIVER_AVAILABLE=1 to its platformio.ini build flags.
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 refines the radio driver integration for the d1l target, specifically addressing how radio functionality is enabled and initialized. It introduces a standardized RADIO_DRIVER_AVAILABLE flag to guard radio-specific code, ensuring that builds without radio hardware compile successfully while full radio functionality is available for enabled configurations. This change also standardizes the LORA_TX_POWER definition and simplifies the global radio variable declaration. ### Highlights * **Conditional Radio Initialization**: Implemented proper RADIO_DRIVER_AVAILABLE guards to conditionally compile and initialize radio-related code, ensuring radio functionality only when explicitly enabled. * **LORA_TX_POWER Fallback**: Added a fallback definition for LORA_TX_POWER to 20 in target.h and target.cpp for builds that do not explicitly define this value. * **Global Radio Variable Declaration**: Modified target.h to directly declare radio_driver as CustomSX1262Wrapper, removing the WRAPPER_CLASS macro and its conditional guard to ensure it's always visible to the linker. * **Repeater Build Configuration**: Enabled radio initialization for the repeater environment by adding RADIO_DRIVER_AVAILABLE=1 to its platformio.ini build flags. <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-26 23:42:58 +02:00
gemini-code-assist[bot] (Migrated from github.com) left a comment
Copy link

Code Review

This pull request effectively implements conditional compilation for radio features using the RADIO_DRIVER_AVAILABLE flag. The change to make the radio_driver global variable always visible to the linker is a solid approach to prevent build issues across different configurations. I have a couple of suggestions to improve code consistency and prevent a potential compilation error.

## Code Review This pull request effectively implements conditional compilation for radio features using the `RADIO_DRIVER_AVAILABLE` flag. The change to make the `radio_driver` global variable always visible to the linker is a solid approach to prevent build issues across different configurations. I have a couple of suggestions to improve code consistency and prevent a potential compilation error.
variants/sensecap_indicator_d1l/target.cpp
@ -6,3 +5,4 @@
// Provide default LORA_TX_POWER for builds that don't define it
#ifndef LORA_TX_POWER
#define LORA_TX_POWER 20
#endif
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-26 23:42:58 +02:00
Copy link

medium

This default definition for LORA_TX_POWER is also present in target.h. Since this file includes target.h, this block is redundant. To avoid duplication and keep configuration in a single, authoritative place (the header), it would be best to remove this block from the .cpp file.

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) This default definition for `LORA_TX_POWER` is also present in `target.h`. Since this file includes `target.h`, this block is redundant. To avoid duplication and keep configuration in a single, authoritative place (the header), it would be best to remove this block from the `.cpp` file.
variants/sensecap_indicator_d1l/target.h
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-26 23:42:58 +02:00
Copy link

high

This is a great change to make radio_driver always available to the linker. Applying this same principle of unconditional declaration for components that are always needed, the declaration of radio_init() (currently on line 54) should also be moved out of its #if RADIO_DRIVER_AVAILABLE guard. The function's implementation in target.cpp now contains non-radio initialization logic (like for clocks) and appears to be designed for unconditional execution. Making its declaration unconditional would prevent potential compilation errors in radio-disabled builds.

![high](https://www.gstatic.com/codereviewagent/high-priority.svg) This is a great change to make `radio_driver` always available to the linker. Applying this same principle of unconditional declaration for components that are always needed, the declaration of `radio_init()` (currently on line 54) should also be moved out of its `#if RADIO_DRIVER_AVAILABLE` guard. The function's implementation in `target.cpp` now contains non-radio initialization logic (like for clocks) and appears to be designed for unconditional execution. Making its declaration unconditional would prevent potential compilation errors in radio-disabled builds.
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!41
No description provided.