refactor(ble): improve bond storage mechanisms - #882
refactor(ble): improve bond storage mechanisms#882heinwessels (heinwessels-philips) wants to merge 20 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors ST BLE bond management to use role-aware bond storage interactors and absolute storage.
Changes:
- Migrates GAP classes to
BondStorageInteractor. - Adapts ST bond storage to typed
GapAddressAPIs. - Simplifies central/peripheral constructor wiring.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
TracingSystemTransportLayerWb.hpp |
Reformats constructor declaration. |
TracingGapPeripheralSt.hpp |
Updates tracing constructor API. |
TracingGapPeripheralSt.cpp |
Updates base construction. |
TracingGapCentralSt.hpp |
Updates tracing constructor API. |
TracingGapCentralSt.cpp |
Updates base construction. |
GapSt.hpp |
Adds interactor-based configuration. |
GapSt.cpp |
Migrates bond operations to the interactor. |
GapPeripheralSt.hpp |
Simplifies constructor declaration. |
GapPeripheralSt.cpp |
Simplifies base construction. |
GapCentralSt.hpp |
Simplifies constructor declaration. |
GapCentralSt.cpp |
Simplifies base construction. |
BondStorageSt.hpp |
Implements the absolute-storage interface. |
BondStorageSt.cpp |
Adds typed bond-address conversion and operations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Max errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 7 | 0 | 0 | 0.1s | ||
| ✅ ACTION | zizmor | 7 | 0 | 0 | 0 | 4.4s | |
| ✅ CPP | clang-format | 15 | 2 | 0 | 0 | 1.63s | |
| ✅ CPP | cppcheck | 15 | 0 | 0 | 0.15s | ||
| ✅ DOCKERFILE | hadolint | 1 | 0 | 0 | 0.02s | ||
| ✅ JSON | jsonlint | 7 | 0 | 0 | 0.08s | ||
| ✅ JSON | prettier | 7 | 2 | 0 | 0 | 1.3s | |
| markdownlint | 6 | 1 | 10 | 0 | 1.63s | ||
| ✅ MARKDOWN | markdown-table-formatter | 6 | 1 | 0 | 0 | 1.11s | |
| ✅ REPOSITORY | betterleaks | yes | no | no | 0.37s | ||
| ✅ REPOSITORY | checkov | yes | no | no | 19.74s | ||
| ✅ REPOSITORY | git_diff | yes | no | no | 0.84s | ||
| ✅ REPOSITORY | grype | yes | no | no | 69.17s | ||
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.01s | ||
| ✅ REPOSITORY | osv-scanner | yes | no | no | 0.84s | ||
| ✅ REPOSITORY | secretlint | yes | no | no | 5.37s | ||
| ✅ REPOSITORY | syft | yes | no | no | 1.74s | ||
| ✅ REPOSITORY | trivy | yes | no | no | 11.45s | ||
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.39s | ||
| ✅ REPOSITORY | trufflehog | yes | no | no | 4.59s | ||
| lychee | 34 | 2 | 0 | 22.36s | |||
| ✅ YAML | prettier | 10 | 1 | 0 | 0 | 1.28s | |
| ✅ YAML | v8r | 10 | 0 | 0 | 4.91s | ||
| ✅ YAML | yamllint | 10 | 0 | 0 | 0.43s |
Detailed Issues
⚠️ SPELL / lychee - 2 errors
📝 Summary
---------------------
🔍 Total..........249
🔗 Unique.........237
✅ Successful.....245
⏳ Timeouts.........0
🔀 Redirected.....114
👻 Excluded.........0
❓ Unknown..........0
🚫 Errors...........2
⛔ Unsupported......2
Errors in .github/ISSUE_TEMPLATE/add-or-update-hal-driver.md
[ERROR] http://mcd.rou.st.com/modules.php?name=mcu (at 28:63) | Connection failed. Check network connectivity and firewall settings
Errors in README.md
[ERROR] https://st.com/ (at 5:97) | HTTP/2 protocol error. Server may not support HTTP/2 properly | Followed 1 redirect. Redirects: https://st.com/ --[301]--> https://www.st.com/
Hint: Followed 114 redirects. You might want to consider replacing redirecting URLs with the resolved URLs. Use verbose mode (`-v`/`-vv`) to see redirection details.
⚠️ MARKDOWN / markdownlint - 10 errors
.github/ISSUE_TEMPLATE/add-or-update-hal-driver.md:10 error MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "How to import a HAL driver"]
CHANGELOG.md:83 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:87 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:94 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:103 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:122 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:140 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:144 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:151 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:162 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
Notices
REPOSITORY_GITLEAKS, REPOSITORY_KICS. See Removed linters to find their replacements.
See detailed reports in MegaLinter artifacts
Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)
- Documentation: Custom Flavors
- Command:
npx mega-linter-runner@10.0.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,ACTION_ZIZMOR,CPP_CPPCHECK,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,JSON_JSONLINT,JSON_PRETTIER,MARKDOWN_MARKDOWNLINT,MARKDOWN_MARKDOWN_TABLE_FORMATTER,REPOSITORY_CHECKOV,REPOSITORY_GIT_DIFF,REPOSITORY_BETTERLEAKS,REPOSITORY_GRYPE,REPOSITORY_LS_LINT,REPOSITORY_OSV_SCANNER,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG,SPELL_LYCHEE,YAML_PRETTIER,YAML_YAMLLINT,YAML_V8R

Show us your support by starring ⭐ the repository
There was a problem hiding this comment.
🟡 Changes recommended
The pinned dependency cannot compile the new calls, and bond matching and failure handling contain correctness defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 4
- Review effort level: Balanced
MiquelJayson-Philips
left a comment
There was a problem hiding this comment.
Once BondStorageAbsolute is renamed don't forget to update this value. Looks good to me.
There was a problem hiding this comment.
🟡 Changes recommended
Identity address types are not preserved or matched correctly, which can remove or misidentify bonds.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
hal_st/middlewares/ble_middleware/AuthoritativeBondStorageSt.cpp:23
- This ignores
address.typeeven thoughBonded_Device_Entry_tsuppliesAddress_TypeandGapAddresstreats the type as part of identity. Public and random identities with identical bytes are distinct, butIsBondStoredwill report either as present andRemoveBondcan delete the wrong entry. Compare the mapped address type as well.
// Note: specifically not checking the address type, since it's not stored in shadow storage currently.
return infra::ContentsEqual(infra::MakeRange(address.address), infra::MakeRange(bondEntry.Address));
- Files reviewed: 16/16 changed files
- Comments generated: 1
- Review effort level: Balanced
Changes GapST to use the new
BondStorageInteractor.BondStorageTiwhere bond roles are not fully supported.. This is because of limitations in our upstream implemention.Depends on:
Related to: