[change] Add 5G signal data on device admin status tab#780
Open
asmodehn wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis change adds support for displaying 5G mobile signal metrics (RSRP, RSRQ, SNR) in the device admin interface. The template is updated to render these metrics as read-only fields when 5G signal data is available. Two test cases validate that the admin UI correctly displays the 5G signal information and that the API properly handles and converts 5G signal values to the expected float format. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_monitoring/device/tests/test_admin.py`:
- Around line 537-539: The assertions only check static labels; update the test
to also assert that the rendered 5G metric values appear and that the legacy
RSSI label is absent: using the existing response variable r1, add
self.assertContains(r1, str(<expected 5G signal_power>)) /
self.assertContains(r1, str(<expected 5G signal_quality>)) /
self.assertContains(r1, str(<expected 5G snr>)) or assert against the 5G
attributes from the fixture used earlier in the test, and add
self.assertNotContains(r1, "Signal Strength (5G)") to ensure RSSI is not shown;
keep using self.assertContains/self.assertNotContains and r1 to locate the
rendered output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 45e194a8-ce77-4a99-ba58-ec946ed9d4f5
📒 Files selected for processing (3)
openwisp_monitoring/device/templates/admin/monitoring/device/change_form.htmlopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
🧰 Additional context used
📓 Path-based instructions (1)
{**/*.sh,**/*.{js,ts,tsx,jsx},**/*.py,**/*.rb,**/*.go,**/*.java,**/*.cs,**/*.cpp,**/*.c}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
🧠 Learnings (5)
📚 Learning: 2026-02-21T01:03:37.822Z
Learnt from: nemesifier
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:827-859
Timestamp: 2026-02-21T01:03:37.822Z
Learning: In Selenium tests (e.g., in openwisp_monitoring/tests/test_selenium.py and similar test files), when testing JS animations on dashboards or elements driven by JavaScript (such as real-time location updates), insert a short sleep (e.g., sleep(0.3)) before WebDriverWait assertions to allow animations to complete and reduce flakiness. Note: use this as a targeted workaround and prefer explicit waits or animation-end checks where possible to avoid relying on fixed delays.
Applied to files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
📚 Learning: 2026-02-21T18:44:28.852Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/api/views.py:263-281
Timestamp: 2026-02-21T18:44:28.852Z
Learning: In openwisp-monitoring, MonitoringIndoorCoordinatesList inherits organization scoping from the parent IndoorCoordinatesList (from openwisp-controller), which uses FilterByParentManaged mixin and filters by location_id in get_queryset(). The child class only overrides the queryset attribute to add monitoring-specific select_related fields; this pattern is safe as long as get_queryset() from the parent is not bypassed. During reviews, verify that MonitoringIndoorCoordinatesList continues to rely on the parent's get_queryset() and that any added select_related fields in the child do not alter the parent's filtering logic.
Applied to files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
📚 Learning: 2026-02-25T18:42:08.825Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:309-317
Timestamp: 2026-02-25T18:42:08.825Z
Learning: In Selenium tests that use ChannelsLiveServerTestCase (e.g., openwisp_monitoring/tests/test_selenium.py), override settings to configure CHANNEL_LAYERS with channels_redis.core.RedisChannelLayer instead of InMemoryChannelLayer. This is required because the live server runs in a separate process from the test process, and InMemoryChannelLayer is per-process only and cannot handle cross-process WebSocket broadcasting needed for real-time location update tests.
Applied to files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
📚 Learning: 2026-03-14T18:39:04.626Z
Learnt from: UltraBot05
Repo: openwisp/openwisp-monitoring PR: 766
File: openwisp_monitoring/utils.py:59-68
Timestamp: 2026-03-14T18:39:04.626Z
Learning: In this repository (openwisp/openwisp-monitoring), the project targets Python 3.10–3.13 as defined by CI. Do not flag backports.zoneinfo as a missing dependency; zoneinfo is a built-in module in all supported Python versions. When reviewing Python code, assume zoneinfo is available and avoid suggesting installation of backports.zoneinfo. If a file imports zoneinfo or uses it for time zone handling, treat it as standard library usage compatible with the supported CI matrix.
Applied to files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
📚 Learning: 2026-03-14T18:39:04.626Z
Learnt from: UltraBot05
Repo: openwisp/openwisp-monitoring PR: 766
File: openwisp_monitoring/utils.py:59-68
Timestamp: 2026-03-14T18:39:04.626Z
Learning: In the openwisp-monitoring project, targets are Linux-based environments. Do not flag a Windows-specific tzdata package as a missing dependency in code reviews for Python files (e.g., openwisp_monitoring/utils.py). If a platform-specific dependency is truly required, document the exception in review guidelines and ensure CI/packaging checks enforce platform constraints rather than manual review.
Applied to files:
openwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_api.py
🔇 Additional comments (2)
openwisp_monitoring/device/templates/admin/monitoring/device/change_form.html (1)
327-346: 5G status rendering matches schema and LTE parity.This block correctly adds 5G-specific fields (RSRP/RSRQ/SNR) and omits RSSI as intended.
openwisp_monitoring/device/tests/test_api.py (1)
808-855: Good 5G API coverage for float coercion and persisted shape.This test validates both static mobile fields and the
5gsignal value conversion to floats with the expected dictionary structure.
Comment on lines
+537
to
+539
| self.assertContains(r1, "Signal Power (5G)") | ||
| self.assertContains(r1, "Signal Quality (5G)") | ||
| self.assertContains(r1, "Signal to noise ratio (5G)") |
There was a problem hiding this comment.
Harden this test by asserting rendered 5G values and RSSI absence.
At Line 537-Line 539, only static labels are asserted. The test should also verify actual values are rendered and that Signal Strength (5G) is not shown.
Suggested test hardening
self.assertContains(r1, "Signal Power (5G)")
self.assertContains(r1, "Signal Quality (5G)")
self.assertContains(r1, "Signal to noise ratio (5G)")
+ self.assertContains(r1, "-75 dBm")
+ self.assertContains(r1, "-8 dB")
+ self.assertContains(r1, "13 dB")
+ self.assertNotContains(r1, "Signal Strength (5G)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/tests/test_admin.py` around lines 537 - 539, The
assertions only check static labels; update the test to also assert that the
rendered 5G metric values appear and that the legacy RSSI label is absent: using
the existing response variable r1, add self.assertContains(r1, str(<expected 5G
signal_power>)) / self.assertContains(r1, str(<expected 5G signal_quality>)) /
self.assertContains(r1, str(<expected 5G snr>)) or assert against the 5G
attributes from the fixture used earlier in the test, and add
self.assertNotContains(r1, "Signal Strength (5G)") to ensure RSSI is not shown;
keep using self.assertContains/self.assertNotContains and r1 to locate the
rendered output.
Test Failures in
|
|
Hi @asmodehn 👋, This is a friendly reminder that this pull request has had no activity for 9 days since changes were requested. We'd love to see this contribution merged! Please take a moment to:
If you're busy or need more time, no worries! Just leave a comment to let us know you're still working on it. Note: within 5 more days, the linked issue will be unassigned to allow other contributors to work on it. Thank you for your contribution! 🙏 |
nemesifier
reviewed
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description of Changes
This adds 5G mobile signal metrics on device admin status tab, duplicating what is done for LTE, minus Signal Strength (rssi - see #779)