-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ (ble): Add Service Device Information #529
✨ (ble): Add Service Device Information #529
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #529 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 103 +1
Lines 1602 1588 -14
=========================================
- Hits 1602 1588 -14
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
dcaa774
to
89cd222
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça me semble bon, juste une question : ça serait pas mieux que le service il utilise un CoreSN
qui lui fournit le serial number plutôt que d'avoir à le setSerialNumber
puisqu'il faudra de toute manière le récupérer à un endroit ?
en attendant d'avoir le CoreSN, on peut lui donner une valeur par défaut genre 00000...000
Ca reste un service BLE, et de la même manière qu'on a décidé de ne pas mettre un CoreBattery ou un BatteryKit dans le service Battery du BLE, je trouve qu'il est important de ne pas mélanger des choses aussi distantes que BLE/Battery ou BLE/SN. En dehors de mettre du CoreSN dans le service BLE, oui on peut faire un CoreSN pour chercher, "synthetiser" et retourner le S/N, mais je ne pense pas que ça s'intègre dans l'objet de cette PR. |
89cd222
to
8b1b929
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
petite typo + proposition de simplification, après c'est good pour moi
9b441d8
to
e0efa2a
Compare
e0efa2a
to
2ab2557
Compare
fe0fbd6
to
af15b9a
Compare
Co-Authored-By: Ladislas de Toldi <ladislas@detoldi.me>
Co-Authored-By: Ladislas de Toldi <ladislas@detoldi.me>
af15b9a
to
57d2cb9
Compare
Kudos, SonarCloud Quality Gate passed! |
No description provided.