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

Feature/fix radio driver guards #35

Merged
okapteinis merged 2 commits from feature/fix-radio-driver-guards into nightly 2025-12-26 20:28:45 +02:00
Conversation 2 Commits 2 Files changed 2 +4 -2
okapteinis commented 2025-12-26 20:28:25 +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 20:28:38 +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 conditional compilation logic for the radio driver, specifically by guarding the instantiation of the MyMesh object to ensure it's only built when the radio driver is available. It also includes minor formatting adjustments to preprocessor directives within the UI task file, enhancing code clarity and maintainability.

Highlights

  • Radio Driver Conditional Compilation: The MyMesh object instantiation in examples/companion_radio/main.cpp is now conditionally compiled using #if RADIO_DRIVER_AVAILABLE. This ensures that the mesh object is only created when a radio driver is present, preventing potential compilation issues or dead code when the driver is not available.
  • Preprocessor Directive Formatting: Adjusted the indentation of #if RADIO_DRIVER_AVAILABLE and #endif directives in examples/companion_radio/ui-new/UITask.cpp for improved code readability and consistency with surrounding code style.
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 conditional compilation logic for the radio driver, specifically by guarding the instantiation of the `MyMesh` object to ensure it's only built when the radio driver is available. It also includes minor formatting adjustments to preprocessor directives within the UI task file, enhancing code clarity and maintainability. ### Highlights * **Radio Driver Conditional Compilation**: The `MyMesh` object instantiation in `examples/companion_radio/main.cpp` is now conditionally compiled using `#if RADIO_DRIVER_AVAILABLE`. This ensures that the mesh object is only created when a radio driver is present, preventing potential compilation issues or dead code when the driver is not available. * **Preprocessor Directive Formatting**: Adjusted the indentation of `#if RADIO_DRIVER_AVAILABLE` and `#endif` directives in `examples/companion_radio/ui-new/UITask.cpp` for improved code readability and consistency with surrounding code style. <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 20:29:36 +02:00
gemini-code-assist[bot] (Migrated from github.com) left a comment
Copy link

Code Review

This pull request aims to add preprocessor guards for radio driver availability. The changes in UITask.cpp are minor stylistic improvements. However, the change in main.cpp is incomplete. It correctly guards the definition of the the_mesh object, but it misses guarding its usages throughout the codebase, as well as other radio-dependent function calls. This will lead to build errors when the radio driver is not available. I've left a critical comment on main.cpp detailing the issues and providing guidance on how to fully implement the required guards.

## Code Review This pull request aims to add preprocessor guards for radio driver availability. The changes in `UITask.cpp` are minor stylistic improvements. However, the change in `main.cpp` is incomplete. It correctly guards the definition of the `the_mesh` object, but it misses guarding its usages throughout the codebase, as well as other radio-dependent function calls. This will lead to build errors when the radio driver is not available. I've left a critical comment on `main.cpp` detailing the issues and providing guidance on how to fully implement the required guards.
examples/companion_radio/main.cpp
@ -98,3 +99,4 @@
, &ui_task
#endif
);
#endif
gemini-code-assist[bot] (Migrated from github.com) commented 2025-12-26 20:29:36 +02:00
Copy link

critical

While this change correctly guards the definition of the_mesh, it is incomplete and will lead to build errors when RADIO_DRIVER_AVAILABLE is 0.

  1. Linker Error: The extern declaration for the_mesh in MyMesh.h is unconditional, and the_mesh is used in several places in this file (setup() and loop()) and other files (UITask.cpp, MyMesh.cpp) without being guarded. This will cause a linker error as the_mesh will be referenced but not defined.

  2. Compilation Errors: Other radio-specific functions are called without guards in setup(). This will cause compilation errors.

    • radio_init() on line 128
    • radio_get_rng_seed() on line 130

To fix this, you need to wrap all code that depends on the radio driver with #if RADIO_DRIVER_AVAILABLE. For example:

// In main.cpp setup()
#if RADIO_DRIVER_AVAILABLE
  if (!radio_init()) { halt(); }

  fast_rng.begin(radio_get_rng_seed());
#endif

// ... later in setup()
#if RADIO_DRIVER_AVAILABLE
  the_mesh.begin(...);
#endif

// ... and in loop()
#if RADIO_DRIVER_AVAILABLE
  the_mesh.loop();
#endif

The extern declaration in MyMesh.h and all other usages of the_mesh and radio_driver across the project should also be guarded to ensure a successful build when RADIO_DRIVER_AVAILABLE is 0.

![critical](https://www.gstatic.com/codereviewagent/critical.svg) While this change correctly guards the definition of `the_mesh`, it is incomplete and will lead to build errors when `RADIO_DRIVER_AVAILABLE` is 0. 1. **Linker Error**: The `extern` declaration for `the_mesh` in `MyMesh.h` is unconditional, and `the_mesh` is used in several places in this file (`setup()` and `loop()`) and other files (`UITask.cpp`, `MyMesh.cpp`) without being guarded. This will cause a linker error as `the_mesh` will be referenced but not defined. 2. **Compilation Errors**: Other radio-specific functions are called without guards in `setup()`. This will cause compilation errors. - `radio_init()` on line 128 - `radio_get_rng_seed()` on line 130 To fix this, you need to wrap all code that depends on the radio driver with `#if RADIO_DRIVER_AVAILABLE`. For example: ```cpp // In main.cpp setup() #if RADIO_DRIVER_AVAILABLE if (!radio_init()) { halt(); } fast_rng.begin(radio_get_rng_seed()); #endif // ... later in setup() #if RADIO_DRIVER_AVAILABLE the_mesh.begin(...); #endif // ... and in loop() #if RADIO_DRIVER_AVAILABLE the_mesh.loop(); #endif ``` The `extern` declaration in `MyMesh.h` and all other usages of `the_mesh` and `radio_driver` across the project should also be guarded to ensure a successful build when `RADIO_DRIVER_AVAILABLE` is 0.
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!35
No description provided.