Honor an explicit -c config path when -s is given

[matutetandil]

Reworked per @jp-bennett's and @caveman99's read: -s should keep auto-loading off, but an explicit -c should still be honored.

In portduinoSetup() the simradio flag is the first branch of the chain that also handles config loading:

if (portduino_config.force_simradio == true) {
    portduino_config.lora_module = use_simradio;
} else if (configPath != nullptr) {
    // load the -c path ...
} else if (access("config.yaml", R_OK) == 0) {
    ...

Because -s short-circuits the whole chain, it swallows the -c branch along with the discovery ones. Skipping discovery is the intent; dropping a path the user typed by hand is not, and it happens with no warning — the file exists, the path is right, the daemon starts fine, the settings just never take effect.

This moves the -s check to sit after the -c branch instead of before it:

if (configPath != nullptr) {
    // explicit -c: always parsed, including under -s
} else if (portduino_config.force_simradio) {
    // -s with no -c: the simulator brings its own defaults, so skip config discovery.
} else if (access("config.yaml", R_OK) == 0) {
    ...

so an explicit path is always parsed, and only the implicit discovery (./config.yaml, /etc/meshtasticd/config.yaml) is skipped under a bare -s.

The radio override then runs after every config source, because -c and the ConfigDirectory entries it can pull in are both able to set Lora.Module, and -s has to win over all of them. Putting it there also fixes --check and --output-yaml, which used to report the configured module rather than the simulated one, since the old override sat behind an early return.

ConfigDirectory needs no special case: it follows whatever config was actually loaded, so a bare -s leaves it empty and the loop does not run.

Verified against the built binary

invocation	config loaded	Lora.Module	MaxNodes
-s (with a ./config.yaml present)	none	sim	200 (default)
-s -c custom.yaml	custom.yaml	sim	77 (from -c)
-c custom.yaml	custom.yaml	sx1262	77
(no flags)	./config.yaml	sx1268	42

--check agrees: bare -s reports "No configuration files were found", -s -c custom.yaml lists custom.yaml and reports Module: sim.

pio run -e native SUCCESS, clang-format clean.

Found while running two simulated nodes with EnableUDP for multicast discovery — the setting was ignored purely because -s was also passed.

Split out of #9749.

Summary by CodeRabbit

• Bug Fixes
  • Explicit configuration files now load correctly when simulated-radio mode is selected.
  • Using simulated-radio mode without a specified configuration file now skips automatic configuration discovery.
  • Simulated-radio mode consistently takes precedence over configured LoRa hardware across normal operation, configuration checks, and YAML output.
  • Configuration settings from all supported locations are processed consistently without unintentionally re-enabling hardware radio mode.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

portduinoSetup() now processes explicit or discovered YAML configuration before applying force_simradio. The simulated-radio override runs after all configuration sources, including ConfigDirectory files.

Changes

Portduino configuration flow

Layer / File(s)	Summary
Load configuration before radio override src/platform/portduino/PortduinoGlue.cpp	Explicit configuration loads in simulated-radio mode. Automatic discovery is skipped when -s is used without -c. After all sources load, force_simradio selects use_simradio.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the primary change: honoring an explicit -c config path when -s is enabled.
Description check	✅ Passed	The description explains the behavior, implementation, rationale, validation results, and affected scenarios in sufficient detail.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/platform/portduino/PortduinoGlue.cpp (1)

334-336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the new comment block to two lines.

Keep only the non-obvious reason that -s must not prevent YAML loading.

As per coding guidelines, C++ comments must be one or two lines maximum and document only non-obvious rationale.

Proposed simplification

-    // Load config YAML first (independent of simradio flag).
-    // The -s flag forces simradio mode but should NOT prevent config loading,
-    // since the YAML may contain non-radio settings (EnableUDP, display, etc.).
+    // Load YAML before applying -s so non-radio settings are preserved.

🤖 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 `@src/platform/portduino/PortduinoGlue.cpp` around lines 334 - 336, Shorten the
comment above YAML configuration loading to two lines, retaining only the
non-obvious rationale that the -s flag must not prevent loading non-radio
settings from the YAML. Remove the redundant statement that config loading is
independent of the simradio flag.

Source: Coding guidelines

🤖 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 `@src/platform/portduino/PortduinoGlue.cpp`:
- Around line 369-373: Move the force_simradio override block using
portduino_config.force_simradio and portduino_config.lora_module until after the
ConfigDirectory loading loop in loadConfig(), but before validation and
emit_yaml(). This ensures directory-based Lora.Module values cannot overwrite
the -s selection.

---

Nitpick comments:
In `@src/platform/portduino/PortduinoGlue.cpp`:
- Around line 334-336: Shorten the comment above YAML configuration loading to
two lines, retaining only the non-obvious rationale that the -s flag must not
prevent loading non-radio settings from the YAML. Remove the redundant statement
that config loading is independent of the simradio flag.

🪄 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: 6151ef0b-1f5f-4c5a-9479-768c86e515d9

📥 Commits

Reviewing files that changed from the base of the PR and between 9e60b23 and 8c6c24f.

📒 Files selected for processing (1)

• src/platform/portduino/PortduinoGlue.cpp

[matutetandil]

Thanks @coderabbitai — good catch, this one was real and I introduced it. Fixed in 47972b2.

loadConfig() sets portduino_config.lora_module whenever the YAML has a Lora.Module key (PortduinoGlue.cpp:938), and the ConfigDirectory loop calls loadConfig() once per .yaml entry. With the override sitting before that loop, a directory entry could put the module back and quietly undo -s.

Worth noting this was not reachable before this PR: config_directory is itself read from the main config file, and the old if (force_simradio) … else if (configPath …) chain meant that with -s no config was ever loaded, so config_directory stayed empty and the loop never ran. Loading the config unconditionally is what opened the window.

The override now runs after the directory loop and before runConfigCheck() / emit_yaml(), so every config source is applied first and the flag wins over all of them. That ordering matters for --check and --output-yaml in particular: the early return in the force_simradio block further down means the radio backend was never actually mis-selected at runtime, but both of those report lora_module before reaching it, so they would have printed a module the daemon was not going to use.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@matutetandil Confirmed. The new order is correct.

All YAML sources now load before force_simradio sets portduino_config.lora_module. This preserves the -s override for runtime selection, --check, and --output-yaml.

🐇 ✅

---

✏️ Learnings added

Learnt from: matutetandil
URL: https://github.com/meshtastic/firmware/pull/11348

Timestamp: 2026-08-03T15:55:50.415Z
Learning: In `src/platform/portduino/PortduinoGlue.cpp`, `loadConfig()` can set `portduino_config.lora_module` when a YAML file contains `Lora.Module`, including YAML files loaded through `ConfigDirectory`. Apply the `force_simradio` (`-s`) override only after all configuration sources, including the `ConfigDirectory` loop, finish loading. Apply it before `runConfigCheck()` and `emit_yaml()` so `--check` and `--output-yaml` report the effective simulated-radio configuration.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

_{You are interacting with an AI system.}

[matutetandil]

Nitpick applied in 2e4ff91 — and it was right on both counts.

AGENTS.md:83 (and .github/copilot-instructions.md:338) does say "Keep code comments minimal - one or two lines, max", and I had a three-line comment there. The follow-up comment I added for the ordering fix was four lines, so I trimmed that one too:

// Load the YAML before applying -s: it also carries non-radio settings
// (EnableUDP, display, GPIO) that have to survive simulated mode.

// Applied after every config source: ConfigDirectory entries can set Lora.Module
// too, and -s must win over all of them, including in --check / --output-yaml.

Both are two lines and keep only the non-obvious why.

[jp-bennett]

I meant to comment on this earlier, but this is actually intended behavior. The -s flag is for simulator use, in which case we really don't want to auto-load yaml. I do agree that the -c flag should be honored.

[matutetandil]

@jp-bennett thanks — that reframes the PR, so let me check which shape you want before I push anything.

I take your point that -s is for simulator use and should not go hunting for YAML on its own. But I do not think the current code implements that intent so much as it happens to produce it, because -s is simply the first arm of an if/else-if chain that also owns config loading:

if (portduino_config.force_simradio == true) {
    portduino_config.lora_module = use_simradio;
} else if (configPath != nullptr) {
    // -c handled here, never reached when -s is set

So -s swallows an explicit -c as well, which is exactly the case you say should be honored. Under your own reading this still needs a fix — just a much narrower one than what I wrote.

What brought me here: a simulated node with EnableUDP under Config: in the YAML never joins the multicast group, because the whole file is skipped and nothing says so. Two simulated nodes therefore cannot find each other. Passing -c explicitly would be a perfectly good answer to that, if -c worked under -s.

The narrow version is roughly this — explicit -c honored, implicit discovery still skipped under -s:

if (configPath != nullptr) {
    // an explicit -c is honored even in simulated mode
    ...
} else if (portduino_config.force_simradio) {
    // -s with no -c: do not go looking for a config
} else if (access("config.yaml", R_OK) == 0) {
    ...

The force_simradio override still has to move below the ConfigDirectory loop, since loadConfig() sets lora_module from Lora.Module and a directory entry could otherwise quietly undo -s. That part stands either way.

Two questions:

1. Narrow version (honor -c only), or the current one (always load, -s overrides the radio)? I am happy either way — the narrow one is closer to what you describe as intended.
2. Should ConfigDirectory follow -c or follow discovery? It is read from the config file, so under the narrow version it only ever populates when -c was given, which seems right but is worth confirming.

@caveman99 — you rebased this branch onto develop earlier today, so flagging that the two of you read the intent differently. I would rather rewrite it once, in whichever direction you both land on, than have it merge as-is if the premise is wrong.

[caveman99]

@matutetandil i fast-forwarded the underlying fork so it doesn't drift too much from develop head. That operationw as not an approval or a rejection, i stand with @jp-bennett assessment. read and parse the -c flag, then if -s is specified force the simradio on.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[matutetandil]

Reworked exactly as asked, @caveman99 — thanks for the call, and for the fast-forward.

-c is now parsed first and always honored; -s moved to sit after that branch, so it only skips the implicit discovery and forces the simradio on:

if (configPath != nullptr) {
    // explicit -c: always parsed, including under -s
} else if (portduino_config.force_simradio) {
    // -s with no -c: the simulator brings its own defaults, so skip config discovery.
} else if (access("config.yaml", R_OK) == 0) {

@jp-bennett's point holds in full: a bare -s loads no YAML at all, same as before. What changes is only that a path the user typed by hand stops being swallowed.

The radio override stays after all config sources, since -c and any ConfigDirectory it pulls in can both set Lora.Module and -s has to beat them. That also answers the ConfigDirectory question I raised earlier — it needs no special case, it just follows whatever config actually loaded, so a bare -s leaves it empty and the loop never runs.

Checked against the built binary rather than by reading:

invocation	config loaded	Lora.Module	MaxNodes
-s (with a ./config.yaml present)	none	sim	200 (default)
-s -c custom.yaml	custom.yaml	sim	77 (from -c)
-c custom.yaml	custom.yaml	sx1262	77
(no flags)	./config.yaml	sx1268	42

Row 1 is the one that matters for your and @jp-bennett's position: the local config.yaml is sitting right there and is deliberately not read. --check agrees — bare -s reports "No configuration files were found", while -s -c custom.yaml lists the file and reports Module: sim.

Rebased onto current develop, still one file. pio run -e native SUCCESS, clang-format clean. Title and description updated, since the old ones described the wider change.