Skip to content

Conversation

ford-jones
Copy link
Contributor

@ford-jones ford-jones commented Sep 30, 2025

Resolves #8139

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Seeed XIAO nRF2840 wio-sx1262

@ford-jones
Copy link
Contributor Author

@ndoo suggested that we simultaneously clear the nodedb.
What do others think? 🤔

@ndoo
Copy link
Contributor

ndoo commented Sep 30, 2025

I think one regression here is that if the user has a custom MQTT topic (perhaps some specific implementation on their own MQTT server), it would get reset by this change. May be inconveniencing that group of users for the convenience of this group of users.

What I assume would fix it is to generate the root topic with the old region and do a string comparison against the new generated root topic

@ford-jones ford-jones changed the title Update MQTT root on lora change Update MQTT root on lora region change Sep 30, 2025
@ford-jones
Copy link
Contributor Author

ford-jones commented Sep 30, 2025

@ndoo this should do it ^ 🙂
Still need to apply this logic to the menu handler then will pull this out of draft

@ndoo
Copy link
Contributor

ndoo commented Sep 30, 2025

@ndoo this should do it ^ 🙂

Still need to apply this logic to the menu handler then will pull this out of draft

Thanks!

Hmm, I may be wrong here, but IIRC even the default generated topic will get saved to userPrefs so this block of code will functionally always enter the first if condition, hence necessitating doing an actual string comparison to determine if the topic is at default.

I.e. the userpref will only be empty if region is UNSET

@ndoo
Copy link
Contributor

ndoo commented Sep 30, 2025

Sorry, I would love to test but am away from a device for 2-3 more days due to a work trip.

@ford-jones
Copy link
Contributor Author

Ah, you're 100% correct ✔️
I've just gotten back home to test this and you're right about the preference being overwritten, I'll push something else up shortly 📡

@ndoo
Copy link
Contributor

ndoo commented Sep 30, 2025 via email

@vidplace7 vidplace7 added the enhancement New feature or request label Sep 30, 2025
@ford-jones ford-jones marked this pull request as ready for review October 1, 2025 05:31
@thebentern thebentern requested a review from Copilot October 1, 2025 20:33
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the MQTT root topic to include the LoRa region name when the region is changed, ensuring proper MQTT topic subscription for different regions. The changes address issue #8139 by automatically updating MQTT configuration when LoRa region settings are modified.

  • Adds logic to update MQTT root topic when LoRa region changes in admin configuration
  • Updates menu handler to include MQTT root topic modification during region selection
  • Updates protobuf submodule to latest version

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/modules/AdminModule.cpp Adds region change detection and MQTT root topic update logic
src/graphics/draw/MenuHandler.cpp Updates LoRa region picker to modify MQTT root topic and handle config changes properly
protobufs Updates submodule to newer commit

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebentern thebentern merged commit 047600d into meshtastic:develop Oct 3, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Default MQTT root topic does not change when LoRa region is changed
4 participants