Skip to content

Conversation

@Will-Panton
Copy link
Contributor

This PR fixes a bug identified in aws_iot.

Since err cannot be both negative AND greater-than-or-equal to a positive value simultaneously, none of the 10 error checks in aws_topics_construct() function were working.

Root Cause:
snprintk() returns:

  • Negative value: Formatting error occurred
  • Value ≥ buffer_size: Output was truncated (buffer too small)

Both conditions indicate errors and should be checked with OR logic, not AND.

This is a pure bug fix that makes existing error handling work as intended. No functional changes to success paths.

@Will-Panton Will-Panton requested a review from a team as a code owner October 31, 2025 17:10
@CLAassistant
Copy link

CLAassistant commented Oct 31, 2025

CLA assistant check
All committers have signed the CLA.

@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 31, 2025
@NordicBuilder
Copy link
Contributor

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

@NordicBuilder NordicBuilder added the external External contribution label Oct 31, 2025
@NordicBuilder NordicBuilder requested a review from a team October 31, 2025 17:10
@SyverHaraldsen
Copy link
Contributor

Hey!
The commit message needs to be updated to follow the commit message guidelines.

https://docs.nordicsemi.com/bundle/ncs-latest/page/zephyr/contribute/guidelines.html#commit_message_guidelines

@rlubos
Copy link
Contributor

rlubos commented Nov 3, 2025

Please use net: lib: aws_iot: prefix for the commit title. The commit also needs some message in the body and the sign-off entry.

@Will-Panton Will-Panton force-pushed the patch-1 branch 2 times, most recently from f0c199c to 541b7c2 Compare November 3, 2025 10:19
@Will-Panton
Copy link
Contributor Author

The commit message has been updated

@SyverHaraldsen
Copy link
Contributor

SyverHaraldsen commented Nov 3, 2025

Compliance fails due to the commit message body not following the contribution guidelines.
The message body text length should be maximum 75 characters. You may use newline to wrap longer lines.

The compliance check also mentions your signed-off line, but to me it looks right. Please verify that it is in accordance with the guidelines below:
https://docs.nordicsemi.com/bundle/ncs-latest/page/zephyr/contribute/guidelines.html#signed-off-by

Edit: Make sure the name and email address you use in the signed-off-by matches the name and email in the Git commit’s author field.

Check AWS MQTT topic construction with OR, as conditions are exclusive.

Signed-off-by: William Panton <william@pantondesign.com>
@Will-Panton
Copy link
Contributor Author

Hopefully these errors have now been fixed.

@rlubos rlubos added the CI-Requested Approves single commit for CI tests on Internal HW label Nov 3, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 3, 2025

CI Information

To view the history of this post, click the 'edited' button above
Build number: 1

Inputs:

Sources:

sdk-nrf: PR head: 22ea079d7c8955ac4f2443bf36b7138167ec7f0e

more details

sdk-nrf:

PR head: 22ea079d7c8955ac4f2443bf36b7138167ec7f0e
merge base: 1cee1ccc3b1833dcbfad9aeb5166e2349b69e788
target head (main): cb5a002a76aa69334f49fe638d471e1679fad3d4
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (1)
subsys
│  ├── net
│  │  ├── lib
│  │  │  ├── aws_iot
│  │  │  │  ├── src
│  │  │  │  │  │ aws_iot.c

Outputs:

Toolchain

Version: cfa6b06338
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:cfa6b06338_e595b21c39

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 397
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-nrf-iot_thingy91
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

Copy link
Contributor

@jtguggedal jtguggedal left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@rlubos rlubos merged commit dce1939 into nrfconnect:main Nov 3, 2025
20 checks passed
@Will-Panton Will-Panton deleted the patch-1 branch November 3, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. CI-Requested Approves single commit for CI tests on Internal HW external External contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants