Skip to content

fix: Subscribe to C8Y topics using QoS 1#3201

Merged
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:fix/c8y-bridge-qos
Oct 30, 2024
Merged

fix: Subscribe to C8Y topics using QoS 1#3201
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:fix/c8y-bridge-qos

Conversation

@didier-wenzek
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek commented Oct 24, 2024

Proposed changes

Subscribe to C8Y topics using QoS 1

  • There is no point to use QoS 2 as the session is cleared on each reconnect
  • The builtin bridge is already using QoS 1

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


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy 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

@didier-wenzek didier-wenzek added theme:c8y Theme: Cumulocity related topics theme:bridge Built-in (Rust) bridge topics labels Oct 24, 2024
@didier-wenzek didier-wenzek marked this pull request as ready for review October 24, 2024 08:41
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 24, 2024 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
528 0 2 528 100 1h32m31.035133s

@reubenmiller
Copy link
Copy Markdown
Contributor

reubenmiller commented Oct 24, 2024

This might help avoid situations where the local bridge is waiting for a response from the server but it never arrives. #3141

reubenmiller
reubenmiller approved these changes Oct 24, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Approved. Makes a lot of sense, and hopefully reduces scenarios like #3141

@didier-wenzek didier-wenzek added this pull request to the merge queue Oct 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2024
@rina23q rina23q mentioned this pull request Oct 24, 2024
Merged
11 tasks
@didier-wenzek
Subscribe to C8Y topics using QoS 1
d5d28f3 
- There is no point to use QoS 2 as the session is cleared on each
  reconnect
- The builtin bridge is already using QoS 1

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
rina23q
rina23q approved these changes Oct 30, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@rina23q rina23q left a comment

Choose a reason for hiding this comment

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

Approved

@didier-wenzek didier-wenzek added this pull request to the merge queue Oct 30, 2024
Merged via the queue into thin-edge:main with commit ecb0fff Oct 30, 2024
@didier-wenzek didier-wenzek deleted the fix/c8y-bridge-qos branch October 30, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reubenmiller reubenmiller reubenmiller approved these changes

@rina23q rina23q rina23q approved these changes

@albinsuresh albinsuresh Awaiting requested review from albinsuresh albinsuresh is a code owner

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

Assignees

No one assigned

Labels

theme:bridge Built-in (Rust) bridge topics theme:c8y Theme: Cumulocity related topics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@didier-wenzek @reubenmiller @rina23q