feat: single connection to either mqtt service or core mqtt #3862

albinsuresh · 2025-11-13T13:30:20Z

Proposed changes

When c8y.mqtt_service.enabled is set to true, just connect to MQTT service endpoint on port 9883 instead of establishing two connections, one to core mqtt and another one to mqtt service. This feature can only be tested against a tenant with the SmartREST proxy feature of MQTT service enabled, so that the same connection can interact with the device over SmartREST or JSON over MQTT.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
Documentation Update (if none of the other choices apply)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3858

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
I ran just format as mentioned in CODING_GUIDELINES
I used just check as mentioned in CODING_GUIDELINES
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

albinsuresh · 2025-11-13T13:40:29Z

crates/core/tedge/src/bridge/c8y.rs

            format!(r#"devicecontrol/notifications in 1 {topic_prefix}/ """#),
            format!(r#"error in 1 {topic_prefix}/ """#),
+            // Custom outgoing topics to MQTT service
+            format!(r#"# out 1 {topic_prefix}/custom/out/ """#),


Used the subtopic: c8y/custom instead of c8y/mqtt based on the differentiation between built-in topics and custom topics.

I don't see custom as clearer. I would be good to find a term stressing that the point is to triage messages to the MQTT service and not C8Y Core.

The c8y docs do use the mqtt term in the pulsar pulsar names, for instance

mqtt/from-device

mqtt/to-device

Using the same topics (with the additional c8y/ prefix) might be ok, though it is a little on the long side.

Using the same topics (with the additional c8y/ prefix) might be ok, though it is a little on the long side.

That mqtt namespace makes sense from a Pulsar perspective as it receives inputs from multiple input channels. But using the same over an MQTT connection just felt redundant. That's why I changed it to custom.

Initially I was considering the c8y/mqtt-svc prefix, which explicitly highlights the "mqtt service" aspect. But decided against it considering the future where MQTT service is the one and only MQTT endpoint, in which case mqtt-svc would also be redundant. So, I thought of focussing on the functional differentiation between built-in topics vs user-provided/custom topics.

I don't see custom as clearer. I would be good to find a term stressing that the point is to triage messages to the MQTT service and not C8Y Core.

Ideally we need a name that highlights the functional difference in the future, but also makes the distinction between core mqtt and mqtt service endpoints clearer during this transition period. The discussed options, none ideal in all senses, are:

c8y/custom

c8y/mqtt-service

c8y/mqtt

If we can't come up with that ideal topic name, let's just vote and pick one between these 3.

My preference would be c8y/mqtt

Resolved by 61db72b

github-actions · 2025-11-13T14:01:52Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
743	0	3	743	100	2h23m14.805707s

codecov · 2025-11-14T07:15:16Z

Codecov Report

❌ Patch coverage is 71.14094% with 43 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/core/tedge_mapper/src/c8y/mapper.rs	0.00%	16 Missing ⚠️
crates/core/tedge/src/cli/connect/command.rs	0.00%	10 Missing ⚠️
crates/core/tedge/src/cli/connect/c8y.rs	0.00%	7 Missing ⚠️
...on/tedge_config/src/tedge_toml/models/host_port.rs	0.00%	4 Missing ⚠️
crates/core/tedge/src/bridge/c8y.rs	96.36%	0 Missing and 4 partials ⚠️
crates/core/tedge/src/cli/connect/aws.rs	0.00%	1 Missing ⚠️
crates/core/tedge/src/cli/connect/azure.rs	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

crates/common/tedge_config/src/tedge_toml/models/c8y.rs

crates/core/tedge/src/cli/connect/c8y.rs

docs/src/operate/c8y/connect-mqtt-service.md

crates/common/tedge_config/src/tedge_toml/tedge_config.rs

docs/src/operate/c8y/connect-mqtt-service.md

jarhodes314 · 2025-11-14T13:43:34Z

crates/core/tedge/src/cli/connect/command.rs

+                let mqtt_svc_host = format!("{}:{}", mqtt_host.host(), MQTT_SVC_TLS_PORT);
+                mqtt_host = HostPort::<MQTT_TLS_PORT>::try_from(mqtt_svc_host)
+                    .map_err(TEdgeConfigError::from)?;


This feels like a very brute-force approach to changing the port, where we generate a completely new string, then parse it. Can we not add a method such as HostPort::set_port to handle this case more cleanly? It might also make the code obvious enough we don't need such a large comment explaining what we're doing here

Resolved by 61db72b

crates/core/tedge_mapper/src/c8y/mapper.rs

tests/RobotFramework/tests/tedge_connect/tedge_connet_mqtt_service.robot

docs/src/operate/c8y/connect-mqtt-service.md

didier-wenzek

I will be happy to approved once addressed the small test issue.

tests/RobotFramework/tests/tedge_connect/tedge_connet_mqtt_service.robot

Connecting to Cumulocity now establishes a single MQTT connection either to the the core MQTT endpoint or the MQTT service, but not both. Connecting to MQTT service requires SmartREST proxy to be enabled on it, so that messaging over SmartREST and JSON over MQTT still works.

didier-wenzek

Approved

albinsuresh temporarily deployed to Test Pull Request November 13, 2025 13:30 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 13, 2025 13:39 — with GitHub Actions Failure

albinsuresh commented Nov 13, 2025

View reviewed changes

albinsuresh force-pushed the feat/mqtt-service-as-primary branch from 6807e74 to 05691cf Compare November 14, 2025 07:03

albinsuresh temporarily deployed to Test Pull Request November 14, 2025 07:03 — with GitHub Actions Inactive

albinsuresh temporarily deployed to Test Auto November 14, 2025 07:13 — with GitHub Actions Inactive

albinsuresh marked this pull request as ready for review November 14, 2025 08:02

albinsuresh requested review from a team, didier-wenzek, jarhodes314, reubenmiller and rina23q as code owners November 14, 2025 08:02

didier-wenzek reviewed Nov 14, 2025

View reviewed changes

jarhodes314 reviewed Nov 14, 2025

View reviewed changes

reubenmiller added the theme:c8y Theme: Cumulocity related topics label Nov 17, 2025

reubenmiller added this to the 1.7.0 milestone Nov 17, 2025

albinsuresh requested a deployment to Test Pull Request November 17, 2025 15:16 — with GitHub Actions Waiting

albinsuresh requested a deployment to Test Pull Request November 18, 2025 06:32 — with GitHub Actions Waiting

didier-wenzek reviewed Nov 18, 2025

View reviewed changes

crates/core/tedge_mapper/src/c8y/mapper.rs Outdated Show resolved Hide resolved

albinsuresh temporarily deployed to Test Pull Request November 19, 2025 14:25 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 19, 2025 14:41 — with GitHub Actions Failure

albinsuresh force-pushed the feat/mqtt-service-as-primary branch from 2c599d0 to 19909b5 Compare November 20, 2025 14:45

albinsuresh temporarily deployed to Test Pull Request November 20, 2025 14:45 — with GitHub Actions Inactive

rina23q reviewed Nov 20, 2025

View reviewed changes

tests/RobotFramework/tests/tedge_connect/tedge_connet_mqtt_service.robot Show resolved Hide resolved

docs/src/operate/c8y/connect-mqtt-service.md Outdated Show resolved Hide resolved

albinsuresh temporarily deployed to Test Auto November 20, 2025 14:57 — with GitHub Actions Inactive

didier-wenzek reviewed Nov 20, 2025

View reviewed changes

tests/RobotFramework/tests/tedge_connect/tedge_connet_mqtt_service.robot Show resolved Hide resolved

albinsuresh force-pushed the feat/mqtt-service-as-primary branch from 19909b5 to e772f1c Compare November 20, 2025 16:45

albinsuresh temporarily deployed to Test Pull Request November 20, 2025 16:45 — with GitHub Actions Inactive

albinsuresh temporarily deployed to Test Auto November 20, 2025 16:55 — with GitHub Actions Inactive

didier-wenzek approved these changes Nov 20, 2025

View reviewed changes

albinsuresh added this pull request to the merge queue Nov 20, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2025

albinsuresh added this pull request to the merge queue Nov 21, 2025

Merged via the queue into thin-edge:main with commit 631de99 Nov 21, 2025
34 checks passed

albinsuresh deleted the feat/mqtt-service-as-primary branch November 28, 2025 10:43

feat: single connection to either mqtt service or core mqtt #3862

feat: single connection to either mqtt service or core mqtt #3862

Uh oh!

Conversation

albinsuresh commented Nov 13, 2025 • edited by reubenmiller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

albinsuresh Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

didier-wenzek Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

reubenmiller Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reubenmiller Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

codecov bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jarhodes314 Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

didier-wenzek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

didier-wenzek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

albinsuresh commented Nov 13, 2025 •

edited by reubenmiller

Loading

albinsuresh Nov 17, 2025 •

edited

Loading

github-actions bot commented Nov 13, 2025 •

edited

Loading

codecov bot commented Nov 14, 2025 •

edited

Loading