NimBLE: Add DIS(Device Information Service) which was missing from port. (IDFGH-4180) #6040
Conversation
|
Neo Kim seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
bot
changed the title
|
Thanks for your contribution. |
|
@neokim Thanks for the PR, I will take a look and update. |
| @@ -642,6 +642,78 @@ | |||
| #endif | |||
|
|
|||
|
|
|||
| /*** @apache-mynewt-nimble/nimble/host/services/dis */ | |||
| #ifndef MYNEWT_VAL_BLE_SVC_DIS_DEFAULT_READ_PERM | |||
| #define MYNEWT_VAL_BLE_SVC_DIS_DEFAULT_READ_PERM (-1) | |||
There was a problem hiding this comment.
Currently we are on nimble-1.2.0 release and yet to migrate to latest nimble 1.3.0 release. Few of the inclusions in latest syscfg.h that you have taken may need to be skipped for IDF specific NimBLE (at least for now). You may refer to esp-nimble-dis for valid inclusions.
There was a problem hiding this comment.
@prasad-alatkar There was no change from nimble-1.2.0 to nimble-1.3.0 on ble_svc_dis.c, and esp-nimble-dis is same as the one of nimble-1.2.0.
Actually, the changes in nimble's syscfg.h are just about configuration. The file is being generated by newt's own configuration tool which is alike to menuconfig. But esp-idf doesn't use it and has no configuration tool for nimble, user has no way to change nimble configuration on his/her esp-idf project. I recommend that esp_nimble_cfg.h has as many inclusion as possible so that user can have a variety of options for nimble.
There was a problem hiding this comment.
Hi, sorry for delayed response. Yes, I understand that we are only adding configuration options and nimble-1.2.0 and nimble-1.3.0 are identical from services/dis point of view. I just looked for instances where BLE_SVC_DIS_DEFAULT_READ_PERM is used, and I could not find any. I am still ok with keeping it in esp_nimble_cfg.h given that it is present in upstream NimBLE (CC: @dhrishi ).
| #endif | ||
|
|
||
| /* Value copied from BLE_SVC_DIS_DEFAULT_READ_PERM */ | ||
| #ifndef MYNEWT_VAL_BLE_SVC_DIS_FIRMWARE_REVISION_READ_PERM |
There was a problem hiding this comment.
Please refer above comment
| #endif | ||
|
|
||
| /* Value copied from BLE_SVC_DIS_DEFAULT_READ_PERM */ | ||
| #ifndef MYNEWT_VAL_BLE_SVC_DIS_HARDWARE_REVISION_READ_PERM |
There was a problem hiding this comment.
Same as above comment.
| #endif | ||
|
|
||
| /* Value copied from BLE_SVC_DIS_DEFAULT_READ_PERM */ | ||
| #ifndef MYNEWT_VAL_BLE_SVC_DIS_MANUFACTURER_NAME_READ_PERM |
There was a problem hiding this comment.
Same as above comments.
|
@prasad-alatkar Thanks for your comments. I’ve just replied. |
espressif-bot
added
Resolution: Done
|
Changes has been part of IDF code already. So closing this . |
Found DIS is missing from porting which already exists in NimBLE and added it to esp-idf.