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

Add conditional compilation guards for radio functions in target.cpp #39

Merged
okapteinis merged 3 commits from feature/fix-build-failures-comprehensive into nightly 2025-12-26 23:40:42 +02:00
Conversation 2 Commits 3 Files changed 2 +3
okapteinis commented 2025-12-26 21:25:01 +02:00 (Migrated from github.com)
Copy link

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

## 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 21:25:11 +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 build process for the sensecap_indicator_d1l variant by implementing conditional compilation for radio-specific functionalities. By wrapping radio functions with preprocessor directives, the changes prevent compilation issues and unnecessary code inclusion on platforms or configurations where a radio driver is not present. This improves the overall modularity and robustness of the codebase, ensuring that radio features are only built when relevant.

Highlights

  • Conditional Compilation: Introduced conditional compilation guards (#if RADIO_DRIVER_AVAILABLE and #endif) around all radio-related functions within target.cpp to ensure these functions are only compiled when a radio driver is explicitly available.
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 build process for the `sensecap_indicator_d1l` variant by implementing conditional compilation for radio-specific functionalities. By wrapping radio functions with preprocessor directives, the changes prevent compilation issues and unnecessary code inclusion on platforms or configurations where a radio driver is not present. This improves the overall modularity and robustness of the codebase, ensuring that radio features are only built when relevant. ### Highlights * **Conditional Compilation**: Introduced conditional compilation guards (`#if RADIO_DRIVER_AVAILABLE` and `#endif`) around all radio-related functions within `target.cpp` to ensure these functions are only compiled when a radio driver is explicitly available. <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 21:26:12 +02:00
gemini-code-assist[bot] (Migrated from github.com) left a comment
Copy link

Code Review

This pull request aims to add conditional compilation guards for radio functions, which is a good step towards making the build more configurable. However, the implementation is incomplete as it only guards the function definitions but not the associated global variables. This will lead to compilation errors when the radio functionality is disabled via the RADIO_DRIVER_AVAILABLE macro. I have added a critical review comment explaining the issue and suggesting a fix.

## Code Review This pull request aims to add conditional compilation guards for radio functions, which is a good step towards making the build more configurable. However, the implementation is incomplete as it only guards the function definitions but not the associated global variables. This will lead to compilation errors when the radio functionality is disabled via the `RADIO_DRIVER_AVAILABLE` macro. I have added a critical review comment explaining the issue and suggesting a fix.
variants/sensecap_indicator_d1l/target.cpp
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-26 21:26:12 +02:00
Copy link

critical

This conditional compilation guard is incomplete. While it correctly wraps the radio function definitions, it fails to wrap the global variables that are specific to the radio functionality.

For example, the definition of radio_driver at line 67 is not guarded:

67: CustomSX1262Wrapper radio_driver(dummy_radio, board);

However, its extern declaration in target.h is guarded by RADIO_DRIVER_AVAILABLE. This will cause a compilation error when RADIO_DRIVER_AVAILABLE is not defined, as there will be a definition without a corresponding declaration.

To fix this, the #if RADIO_DRIVER_AVAILABLE block should be extended to also cover all radio-specific global variables, such as custom_hal, radio, radio_module, dummy_radio, and radio_driver. The guard should likely start before the definitions of these variables (e.g., around line 60) and encompass all radio-related code.

![critical](https://www.gstatic.com/codereviewagent/critical.svg) This conditional compilation guard is incomplete. While it correctly wraps the radio function definitions, it fails to wrap the global variables that are specific to the radio functionality. For example, the definition of `radio_driver` at line 67 is not guarded: ```cpp 67: CustomSX1262Wrapper radio_driver(dummy_radio, board); ``` However, its `extern` declaration in `target.h` is guarded by `RADIO_DRIVER_AVAILABLE`. This will cause a compilation error when `RADIO_DRIVER_AVAILABLE` is not defined, as there will be a definition without a corresponding declaration. To fix this, the `#if RADIO_DRIVER_AVAILABLE` block should be extended to also cover all radio-specific global variables, such as `custom_hal`, `radio`, `radio_module`, `dummy_radio`, and `radio_driver`. The guard should likely start before the definitions of these variables (e.g., around line 60) and encompass all radio-related code.
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!39
No description provided.