Nz sf region - #985
Conversation
Adds a new RegionCode enum value NZ_SF = 38 that uses the same frequency band as ANZ, intended for the Short Fast modem preset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CZ7bDuPL67jJS1D7RV9oB
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CZ7bDuPL67jJS1D7RV9oB
|
|
📝 WalkthroughWalkthroughUpdates protocol schemas with the ChangesProtocol schema updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meshtastic/config.proto`:
- Around line 993-998: Increase the max_count for
LoRaRegionPresetMap.region_groups in mesh.options from 38 to 39 to accommodate
RegionCode.NZ_924, then regenerate the protobuf bindings.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 489e2958-1f85-4bc1-be6d-625d60b197a4
📒 Files selected for processing (1)
meshtastic/config.proto
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meshtastic/config.proto`:
- Around line 995-998: Update LoRaRegionPresetMap.region_groups in mesh.proto to
include NZ_915 with default_preset set to SHORT_FAST, and increase the
region_groups entry limit in mesh.options if necessary to accommodate the new
mapping. Ensure clients can validate unsupported preset combinations for this
region.
In `@meshtastic/mesh.proto`:
- Line 1819: Coordinate the rx_time explicit-presence change across all
consumers: update writers and readers to use the generated presence API,
regenerate NanoPB bindings so has_rx_time is available, and add round-trip
coverage distinguishing an absent value from a present value of 0. Ensure these
updates are complete before merging the protobuf schema change.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ff867d8-fc69-4396-8792-d70322a6a1a9
📒 Files selected for processing (3)
meshtastic/config.protomeshtastic/mesh.optionsmeshtastic/mesh.proto
| * New Zealand 915MHz-928MHz | ||
| * Same band as ANZ, for use with the Short Fast modem preset | ||
| */ | ||
| NZ_915 = 38; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 '\bNZ_915\b|\bSHORT_FAST\b|\bLoRaRegionPresetMap\b|\bregion_groups\b' . || trueRepository: meshtastic/protobufs
Length of output: 5823
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== config.proto around NZ_915 and presets =="
sed -n '960,1060p' meshtastic/config.proto
echo
echo "== mesh.proto LoRaPresetGroup block =="
sed -n '245,330p' meshtastic/config.proto | sed -n '2790,2840p' meshtastic/mesh.proto
echo
echo "== all RegionCode values =="
python3 - <<'PY'
from pathlib import Path
import re
p = Path('meshtastic/config.proto')
text = p.read_text()
m = re.search(r"enum\s+RegionCode\s*\{([^}]+)\}", text, re.S)
if not m:
print("REGION_CODE_NOT_FOUND")
raise SystemExit
for line in m.group(1).splitlines():
s=line.strip()
if not s or s.startswith('/*') or s.startswith('*'):
continue
print(s)
PY
echo
echo "== repo-wide NZ_915 occurrences =="
rg -n '\bNZ_915\b' . || true
echo
echo "== preset/region map definitions in proto files =="
rg -n 'LoRaRegionPresetMap|LoRaRegionPresets|LoRaPresetGroup|region_groups|LoRaVersion' meshtastic meshtastic_proto meshtastic.proto *.proto 2>/dev/null || trueRepository: meshtastic/protobufs
Length of output: 6450
Add NZ_915 to the region-preset map contract.
NZ_915 = 38 is a valid new region value, but meshtastic/mesh.proto still has no LoRaRegionPresetMap.region_groups entry for it, and meshtastic/mesh.options caps only the map at 39 entries. Add the NZ_915 preset-group mapping with default_preset = SHORT_FAST so clients can reject unsupported preset combinations, or leave clear documentation that region_groups has no entry for this region.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@meshtastic/config.proto` around lines 995 - 998, Update
LoRaRegionPresetMap.region_groups in mesh.proto to include NZ_915 with
default_preset set to SHORT_FAST, and increase the region_groups entry limit in
mesh.options if necessary to accommodate the new mapping. Ensure clients can
validate unsupported preset combinations for this region.
| * clock becomes available, before the phone ever sees it - "absent" is not guaranteed | ||
| * permanent, only "not yet known at last observation". | ||
| */ | ||
| optional fixed32 rx_time = 7; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Expect: the generator and schema-compatibility policy are identified.
rg --hidden -n -C 5 \
'protoc|nanopb|proto3_optional|breaking|cardinality' \
-g '!.git' .
# Expect: all rx_time writers and readers use presence-aware APIs.
rg --hidden -n -C 6 \
'rx_time|has_rx_time|HasField.*rx_time|ClearField.*rx_time' \
-g '!.git' .Repository: meshtastic/protobufs
Length of output: 35149
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== buf.yaml =="
cat -n buf.yaml
echo "== workflow breaking/lint snippets =="
sed -n '1,80p' .github/workflows/pull_request.yml | cat -n
echo "== mesh proto rx_time and ServiceEnvelope context =="
sed -n '1730,1825p;2130,2195p' meshtastic/mesh.proto | cat -n
echo "== top-level proto options for proto3_optional / explicit presence config =="
rg -n -C 3 'proto3_optional|syntax = "proto3"|breaking|except:' meshtastic *.proto options build.gradle* .gradle* 2>/dev/null || trueRepository: meshtastic/protobufs
Length of output: 12718
🌐 Web query:
NanoPB proto3 optional generated has field has_rx_time
💡 Result:
In NanoPB, a has_ field (such as has_rx_time) is generated when a field is explicitly marked as optional in your .proto file, even when using proto3 syntax [1][2]. Under standard proto3 behavior, singular fields do not typically generate a has_ flag because they rely on default values [2][3]. However, if you use the explicit optional keyword introduced in Protocol Buffers 3.12, NanoPB will recognize this and generate a corresponding has_ boolean field in the C code [2][4]. This has_ field allows you to explicitly track whether the value is present or absent, regardless of whether its value is the default (e.g., zero) [1][4]. If your generated code contains has_rx_time, it confirms that the field rx_time was defined with the optional label in your .proto file (e.g., optional uint32 rx_time = 1;) [2][4]. In projects like Meshtastic, this pattern is intentionally used to distinguish between a valid time value and a missing or "not-present" state [4].
Citations:
- 1: Option to always encode proto3 fields, even if they have their default value nanopb/nanopb#878
- 2: https://chromium.googlesource.com/external/github.com/nanopb/nanopb/+/master/generator/nanopb_generator.py
- 3: Suppression of has_* fields for protobuf 3 nanopb/nanopb#182
- 4: Arrival time fix perhaps firmware#11274
Coordinate the explicit-presence migration with downstream generated bindings.
meshtastic/mesh.proto:1819 is valid explicit presence, but this breaks existing implicit-presence clients and generated APIs. Update writers/readers with the generated presence API, regenerate NanoPB bindings with has_rx_time, and add round-trip tests for absent vs present 0 before the breaking protobuf change is merged.
🧰 Tools
🪛 GitHub Check: build
[failure] 1819-1819:
Field "7" with name "rx_time" on message "MeshPacket" changed cardinality from "optional with implicit presence" to "optional with explicit presence".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@meshtastic/mesh.proto` at line 1819, Coordinate the rx_time explicit-presence
change across all consumers: update writers and readers to use the generated
presence API, regenerate NanoPB bindings so has_rx_time is available, and add
round-trip coverage distinguishing an absent value from a present value of 0.
Ensure these updates are complete before merging the protobuf schema change.
Source: Linters/SAST tools
3 participants
Adds a new region specifically for New Zealand to allow the ShortFast preset generally in use there to be the "default" from the start. ANZ is still available for legacy users.
Checklist before merging
Summary by CodeRabbit