BLE: Data sent event parameter is useless information

[AGlass0fMilk]

Description of defect

The information passed to the onDataSent callback handler is a static 1 and is therefore useless information...

mbed-os/connectivity/FEATURE_BLE/source/cordio/source/GattServerImpl.cpp

Lines 1514 to 1517 in ad40b1b

	case GattServerEvents::GATT_EVENT_DATA_SENT:
	// Called every time a notification or indication has been sent
	handleDataSentEvent(1);
	break;

It would be better to include the GattAttribute::Handle_t of the specific characteristic for which the event is related to.

Target(s) affected by this defect ?

All

Toolchain(s) (name and version) displaying this defect ?

arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 9-2019-q4-major) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]

What version of Mbed-os are you using (tag or sha) ?

ad40b1b

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

appdirs 1.4.3
asn1ate 0.6.0
attrs 19.3.0
beautifulsoup4 4.6.3
cbor 1.0.0
certifi 2019.11.28
cffi 1.14.1
chardet 3.0.4
Click 7.0
cmsis-pack-manager 0.2.10
cobs 1.1.4
colorama 0.3.9
crc16 0.1.1
crccheck 0.6
cryptography 2.9.2
cycler 0.10.0
ecdsa 0.15
elftools 0.1.0.dev0
fasteners 0.15
future 0.16.0
futures 3.1.1
fuzzywuzzy 0.18.0
hidapi 0.9.0.post2
icetea 1.2.4
idna 2.7
imgtool 1.6.0
importlib-metadata 1.6.0
intelhex 2.2.1
Jinja2 2.10.3
jsonmerge 1.7.0
jsonschema 2.6.0
junit-xml 1.8
kiwisolver 1.2.0
lockfile 0.12.2
Logbook 1.5.3
manifest-tool 1.5.2
MarkupSafe 1.1.1
matplotlib 3.3.0
mbed-cli 1.10.4
mbed-cloud-sdk 2.0.8
mbed-flasher 0.10.1
mbed-greentea 1.7.4
mbed-host-tests 1.5.10
mbed-ls 1.7.12
mbed-os-tools 0.0.15
milksnake 0.1.5
monotonic 1.5
numpy 1.19.1
pc-ble-driver-py 0.14.2
Pillow 7.2.0
pip 20.2
pkg-resources 0.0.0
prettytable 0.7.2
protobuf 3.5.2.post1
psutil 5.6.6
pyasn1 0.2.3
pycparser 2.20
pycryptodome 3.9.8
pyelftools 0.25
pyparsing 2.4.7
pyrsistent 0.16.0
pyserial 3.4
python-dateutil 2.8.1
python-dotenv 0.14.0
pyusb 1.0.2
PyYAML 4.2b1
requests 2.20.1
semver 2.10.2
setuptools 46.1.3
six 1.12.0
soupsieve 2.0
trollius 2.1.post2
urllib3 1.24.2
wheel 0.34.2
wrapt 1.12.1
yattag 1.13.2
zipp 3.1.0

How is this defect reproduced ?

Try to use onDataSent in BLE stack

[AGlass0fMilk]

@pan- @paul-szczepanek-arm

[pan-]

On cordio it is always a single packet that is reported by the handler so 1 is hardcoded.

We need to add two information to the event to make it more useful:

• attribute handle
• connection handle

I commented yesterday on an issue asking for the connection handle.

We will issue a PR to improve the situation in the upcoming Sprint.

[AGlass0fMilk]

I am working on a PR right now. Thanks for the quick response.

Can you link to the issue you are referring to?

[AGlass0fMilk]

@pan-

I also think the onUpdatesEnabled/Disabled/onConfirmationReceived should be changed into CallChains to be consistent with the API expected of the other on[...] callback registering functions.

I will issue a PR for this as well.

[pan-]

Here's the issue I'm referring to: #12602

I also think the onUpdatesEnabled/Disabled/onConfirmationReceived should be changed into CallChains to be consistent with the API expected of the other on[...] callback registering functions.

Many thanks for the PR and thanks for thinking of the consistency.
Thinking of the future is important too. Could you add these these new events into the EventHandler and define a data type for the event so it can be extended if the Bluetooth specification evolves ?

[AGlass0fMilk]

Here's the issue I'm referring to: #12602

I also think the onUpdatesEnabled/Disabled/onConfirmationReceived should be changed into CallChains to be consistent with the API expected of the other on[...] callback registering functions.

Many thanks for the PR and thanks for thinking of the consistency.
Thinking of the future is important too. Could you add these these new events into the EventHandler and define a data type for the event so it can be extended if the Bluetooth specification evolves ?

@pan-

Yes, I'm doing that as we type.

I'm planning to do two PRs --

1.) one to change the parameters passed into the above-mentioned functions and
2.) another to change the other handlers to CallChains so a typical GattService can register its own callbacks without overwriting the callbacks of other application modules.

I was first planning to use the same datatype for all of these functions (onSent, onUpdatesEnabled...) but it may be more flexible for the future to use separate datatypes for each?

[pan-]

Using separate data type is more flexible. For instance, in onUpdatesEnabled we can pass if notification or indication was enabled.

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2411

[pan-]

I believe this can be closed as #13734 has been merged.