Skip to content

fix: do not persist temporary callsign mapping for unknown devices#84

Open
clayauld wants to merge 1 commit intomainfrom
fix-temporary-callsign-persistence-1121023223483368229
Open

fix: do not persist temporary callsign mapping for unknown devices#84
clayauld wants to merge 1 commit intomainfrom
fix-temporary-callsign-persistence-1121023223483368229

Conversation

@clayauld
Copy link
Owner

@clayauld clayauld commented Mar 6, 2026

Prevents hardware IDs of unknown devices from being permanently assigned as callsigns in the database, allowing proper nodeinfo updates to be persisted later. Also ensures configured devices use the configuration file as the sole source of truth by avoiding redundant persistence.


PR created automatically by Jules for task 1121023223483368229 started by @clayauld

Co-authored-by: clayauld <2290528+clayauld@users.noreply.github.com>
@google-labs-jules
Copy link
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, 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 callsign mapping logic within the gateway application to enhance data integrity and consistency. It establishes the configuration file as the definitive source of truth for configured devices, eliminating redundant database writes. Additionally, it prevents the persistence of temporary callsign mappings for unknown devices, thereby avoiding potential conflicts with future, accurate nodeinfo updates.

Highlights

  • Configured Device Callsign Persistence: Prevented the redundant persistence of callsign mappings for devices that are explicitly defined in the configuration, ensuring the configuration file remains the single source of truth for these devices.
  • Unknown Device Callsign Handling: Ensured that temporary callsign mappings generated for unknown devices are not permanently stored in the database, which allows for proper nodeinfo updates to be persisted later.
  • Test Coverage: Added a new test case to specifically verify that temporary callsign mappings for unknown devices are not persisted, and updated an existing test to reflect the non-persistence of configured device callsigns.
Changelog
  • src/gateway_app.py
    • Removed the call to _persist_callsign_mapping when a configured device ID is found, ensuring configuration is the sole source of truth.
    • Updated the debug logging message to reflect that persistent callsign mapping is skipped for configured hardware IDs.
  • tests/test_gateway_app.py
    • Modified test_process_nodeinfo_message to assert that configured callsigns are no longer persisted to the callsign_mapping (database representation).
    • Added a new test, test_unknown_device_callsign_not_persisted, to verify that temporary callsigns for unknown devices are not stored in the cache or persistence layer.
Activity
  • PR created automatically by Jules for task 1121023223483368229, started by @clayauld.
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.

Footnotes

  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.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

🤖 Automated Code Review

🔍 Linting Results Summary

✅ Black (Code Formatting) - PASSED

❌ Flake8 (Code Quality)

tests/test_gateway_app.py:288:89: E501 line too long (91 > 88 characters)

Fix: Address the code quality issues shown above.

✅ isort (Import Sorting) - PASSED

✅ MyPy (Type Checking) - PASSED

✅ Bandit (Security) - PASSED

🚨 Action Required

Some linting checks failed. Please fix the issues above and push your changes.


This comment was automatically generated by GitHub Actions. Check the Actions tab for detailed logs.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

📊 Pull Request Analysis Report

🧪 Test Results

Metric Value
Total Tests 112
✅ Passed 112
❌ Failed 0
⏭️ Skipped 0
💥 Errors 0
📈 Success Rate 100.0%

📋 Coverage Report

Metric Value
Overall Coverage 89.6%
Total Lines 770
Covered Lines 690
Missing Lines 80

📚 Documentation Report

Metric Value
Documentation Coverage 100.0%
Status ✅ Up to date

📁 File Coverage Breakdown

  • config/__init__.py: 100.0%
  • config/config.py: 95.3%
  • src/__init__.py: 100.0%
  • src/caltopo_reporter.py: 95.0%
  • src/gateway.py: 84.4%
  • src/gateway_app.py: 83.5%
  • src/mqtt_client.py: 92.0%
  • src/persistent_dict.py: 90.9%
  • src/utils.py: 100.0%

🎯 Recommendations

✅ Excellent Code Quality

  • Great work maintaining high test pass rate and coverage!

📊 Analysis generated by GitHub Actions • View detailed test report

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses the issue of improperly persisting callsigns for both configured and unknown devices. The change to stop persisting callsigns for configured devices from _process_nodeinfo_message correctly establishes the configuration file as the single source of truth. The associated test updates and the new test case for unknown devices are thorough and validate the intended behavior effectively. I've noted one potential area for improvement in a related function regarding what appears to be unreachable code, which could enhance maintainability.

app.callsign_mapping.clear()

# Call the method
callsign = app._get_or_create_callsign(hardware_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While reviewing the changes, I noticed a potential issue in the _get_or_create_callsign method being tested here. The code block from lines 423-429 in src/gateway_app.py appears to be unreachable.

The logic to reach this block requires a device to be 'known' (i.e., hardware_id is in self.configured_devices), but for self.config.get_node_device_id(hardware_id) to have returned None. Given the implementation of get_node_device_id and the NodeMapping model where device_id is a required field, this scenario seems impossible.

This suggests there might be some dead code or a misunderstanding in the logic for handling 'known' vs 'unknown' devices. While not directly part of this PR's changes, it's worth investigating to improve code clarity and maintainability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant