Skip to content

BLE: Cordio bugfix fixed size writes under size must be allowed #13283

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

Merged

Conversation

paul-szczepanek-arm
Copy link
Member

Summary of changes

Fixed size ATT writes within size must be allowed as described in CORE SPECIFICATION Version 5.2, Vol 3, Part F, 3.4.5.1:

If the attribute value has a fixed length and the Attribute Value parameter length is less than or equal to the length of the attribute value, the octets of the attribute value parameter length shall be written; all other octets in this attribute value shall be unchanged.

Impact of changes

Migration actions required

Documentation

None


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Reviewers


@paul-szczepanek-arm
Copy link
Member Author

This is tested and verified by the new tests:

============================= test session starts ==============================
platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.1, pluggy-0.13.0 -- /usr/bin/python3.8
cachedir: .pytest_cache
rootdir: /home/paul/gh/tests-paul/ble-tests, inifile: pytest.ini
plugins: mock-1.10.4, pep8-1.0.6
collecting ... collected 1 item

test_descriptor_read_write.py::test_client_fail_write_descriptor_when_larger_than_fixed_size 
------------------------------ live log teardown -------------------------------
14:37:18.756   INFO   -->|peripheral| ble shutdown
14:37:18.761   INFO   <--|peripheral| {"status": 0}
14:37:18.762   INFO   <--|peripheral| retcode: 0
14:37:18.763   INFO   -->|peripheral| echo on
14:37:18.766   INFO   <--|peripheral| ECHO is on
14:37:18.767   INFO   <--|peripheral| retcode: 0
14:37:18.768   INFO   -->|peripheral| set --vt100 on
14:37:18.772   INFO   <--|peripheral| 
14:37:18.773   INFO   <--|peripheral| retcode: 0
14:37:18.773   INFO   Stopping "peripheral" runner...
14:37:18.774   INFO   -->|peripheral| set retcode false
14:37:18.808   INFO   <--|peripheral| />set retcode false
14:37:18.873   INFO   -->|central| ble shutdown
14:37:18.878   INFO   <--|central| {"status": 0}
14:37:18.879   INFO   <--|central| retcode: 0
14:37:18.879   INFO   -->|central| echo on
14:37:18.883   INFO   <--|central| ECHO is on
14:37:18.884   INFO   <--|central| retcode: 0
14:37:18.885   INFO   -->|central| set --vt100 on
14:37:18.889   INFO   <--|central| 
14:37:18.890   INFO   <--|central| retcode: 0
14:37:18.891   INFO   Stopping "central" runner...
14:37:18.891   INFO   -->|central| set retcode false
14:37:18.925   INFO   <--|central| />set retcode false


=========================== 1 passed in 5.51 seconds ===========================

Process finished with exit code 0

-------------------------------- live log setup --------------------------------
14:37:13.562   INFO   Starting runner threads for "central"
14:37:14.082   INFO   <--|central| ARM Ltd
14:37:14.677   INFO   -->|central| set --retcode true
14:37:14.716   INFO   <--|central| />set --retcode true
14:37:14.717   INFO   <--|central| retcode: 0
14:37:14.722   INFO   -->|central| echo off
14:37:14.736   INFO   <--|central| />echo off
14:37:14.738   INFO   <--|central| ECHO is off
14:37:14.739   INFO   <--|central| retcode: 0
14:37:14.739   INFO   -->|central| set --vt100 off
14:37:14.743   INFO   <--|central| retcode: 0
14:37:14.743   INFO   -->|central| ble init
14:37:14.761   INFO   <--|central| {"status": 0}
14:37:14.762   INFO   <--|central| retcode: 0
14:37:14.764   INFO   Starting runner threads for "peripheral"
14:37:15.290   INFO   <--|peripheral| ARM Ltd
14:37:15.877   INFO   -->|peripheral| set --retcode true
14:37:15.914   INFO   <--|peripheral| />set --retcode true
14:37:15.915   INFO   <--|peripheral| retcode: 0
14:37:15.916   INFO   -->|peripheral| echo off
14:37:15.929   INFO   <--|peripheral| />echo off
14:37:15.930   INFO   <--|peripheral| ECHO is off
14:37:15.931   INFO   <--|peripheral| retcode: 0
14:37:15.932   INFO   -->|peripheral| set --vt100 off
14:37:15.936   INFO   <--|peripheral| retcode: 0
14:37:15.936   INFO   -->|peripheral| ble init
14:37:15.954   INFO   <--|peripheral| {"status": 0}
14:37:15.955   INFO   <--|peripheral| retcode: 0
-------------------------------- live log call ---------------------------------
14:37:15.957   INFO   -->|peripheral| gattServer declareService 65531
14:37:15.963   INFO   <--|peripheral| {"status": 0}
14:37:15.964   INFO   <--|peripheral| retcode: 0
14:37:15.965   INFO   -->|peripheral| gattServer declareCharacteristic 57007
14:37:15.971   INFO   <--|peripheral| {"status": 0}
14:37:15.972   INFO   <--|peripheral| retcode: 0
14:37:15.973   INFO   -->|peripheral| gattServer setCharacteristicProperties notify indicate
14:37:15.980   INFO   <--|peripheral| {"status": 0}
14:37:15.982   INFO   <--|peripheral| retcode: 0
14:37:15.982   INFO   -->|peripheral| gattServer declareDescriptor 57005
14:37:15.989   INFO   <--|peripheral| {"status": 0}
14:37:15.990   INFO   <--|peripheral| retcode: 0
14:37:15.990   INFO   -->|peripheral| gattServer setDescriptorValue 00112233
14:37:15.997   INFO   <--|peripheral| {"status": 0}
14:37:15.998   INFO   <--|peripheral| retcode: 0
14:37:15.998   INFO   -->|peripheral| gattServer setDescriptorVariableLength false
14:37:16.005   INFO   <--|peripheral| {"status": 0}
14:37:16.006   INFO   <--|peripheral| retcode: 0
14:37:16.007   INFO   -->|peripheral| gattServer setDescriptorMaxLength 4
14:37:16.014   INFO   <--|peripheral| {"status": 0}
14:37:16.015   INFO   <--|peripheral| retcode: 0
14:37:16.015   INFO   -->|peripheral| gattServer commitService
14:37:16.049   INFO   <--|peripheral| {"status": 0,"result": {"UUID": 65531,"handle": 11,"characteristics": [{"UUID": 57007,"value_handle": 13,"properties": ["notify","indicate"],"length": 0,"max_length": 0,"has_variable_length": true,"value": "","descriptors": [{"UUID": 57005,"handle": 14,"length": 4,"max_length": 4,"has_variable_length": false,"value": "00112233"}]}]}}
14:37:16.050   INFO   <--|peripheral| retcode: 0
14:37:16.051   INFO   -->|peripheral| advDataBuilder clear
14:37:16.056   INFO   <--|peripheral| {"status": 0}
14:37:16.057   INFO   <--|peripheral| retcode: 0
14:37:16.058   INFO   -->|peripheral| advDataBuilder setFlags LE_GENERAL_DISCOVERABLE
14:37:16.066   INFO   <--|peripheral| {"status": 0}
14:37:16.067   INFO   <--|peripheral| retcode: 0
14:37:16.067   INFO   -->|peripheral| advDataBuilder setFlags BREDR_NOT_SUPPORTED
14:37:16.076   INFO   <--|peripheral| {"status": 0}
14:37:16.077   INFO   <--|peripheral| retcode: 0
14:37:16.077   INFO   -->|peripheral| gap applyAdvPayloadFromBuilder 0
14:37:16.084   INFO   <--|peripheral| {"status": 0}
14:37:16.085   INFO   <--|peripheral| retcode: 0
14:37:16.086   INFO   -->|peripheral| advParams setType CONNECTABLE_UNDIRECTED
14:37:16.094   INFO   <--|peripheral| {"status": 0}
14:37:16.095   INFO   <--|peripheral| retcode: 0
14:37:16.096   INFO   -->|peripheral| gap setAdvertisingParameters 0
14:37:16.102   INFO   <--|peripheral| {"status": 0}
14:37:16.103   INFO   <--|peripheral| retcode: 0
14:37:16.104   INFO   -->|peripheral| gap getAddress
14:37:16.114   INFO   <--|peripheral| {"status": 0,"result": {"address_type": "RANDOM","address": "DF:08:18:BF:DD:56"}}
14:37:16.115   INFO   <--|peripheral| retcode: 0
14:37:16.116   INFO   -->|peripheral| gap startAdvertising 0 0 0
14:37:16.123   INFO   <--|peripheral| {"status": 0}
14:37:16.124   INFO   <--|peripheral| retcode: 0
14:37:16.125   INFO   -->|peripheral| gap waitForConnection 10000
14:37:16.126   INFO   -->|central| gap connect RANDOM DF:08:18:BF:DD:56
14:37:16.793   INFO   <--|central| {"status": 0,"result": {"status": "BLE_ERROR_NONE","peer_address_type": "RANDOM","peer_address": "DF:08:18:BF:DD:56","interval": 56,"latency": 0,"supervision_timeout": 100,"connection_handle": 1,"own_role": "CENTRAL","master_clock_accuracy": 0}}
14:37:16.795   INFO   <--|central| retcode: 0
14:37:16.796   INFO   <--|peripheral| {"status": 0,"result": {"status": "BLE_ERROR_NONE","peer_address_type": "RANDOM","peer_address": "DA:50:66:92:87:FA","interval": 56,"latency": 0,"supervision_timeout": 100,"connection_handle": 1,"own_role": "PERIPHERAL","master_clock_accuracy": 0}}
14:37:16.797   INFO   <--|peripheral| retcode: 0
14:37:16.806   INFO   <--|peripheral| <<< {"type": "event","name": "advertising_end","value": {"advertising_handle": 0,"completed_events": 0,"is_connected": true,"connection_handle": 0}}
14:37:16.897   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AA
14:37:17.063   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"length": 0,"write_operation_type": "OP_WRITE_REQ","data": ""}}
14:37:17.064   INFO   <--|central| retcode: 0
14:37:17.065   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:17.203   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "AA112233"}}
14:37:17.204   INFO   <--|central| retcode: 0
14:37:17.205   INFO   -->|peripheral| gattServer read 14
14:37:17.211   INFO   <--|peripheral| {"result": "AA112233","status": 0}
14:37:17.212   INFO   <--|peripheral| retcode: 0
14:37:17.213   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AAAA
14:37:17.343   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"length": 0,"write_operation_type": "OP_WRITE_REQ","data": ""}}
14:37:17.345   INFO   <--|central| retcode: 0
14:37:17.345   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:17.483   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "AAAA2233"}}
14:37:17.484   INFO   <--|central| retcode: 0
14:37:17.485   INFO   -->|peripheral| gattServer read 14
14:37:17.491   INFO   <--|peripheral| {"result": "AAAA2233","status": 0}
14:37:17.492   INFO   <--|peripheral| retcode: 0
14:37:17.493   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AAAAAA
14:37:17.623   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"length": 0,"write_operation_type": "OP_WRITE_REQ","data": ""}}
14:37:17.625   INFO   <--|central| retcode: 0
14:37:17.625   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:17.763   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "AAAAAA33"}}
14:37:17.764   INFO   <--|central| retcode: 0
14:37:17.765   INFO   -->|peripheral| gattServer read 14
14:37:17.771   INFO   <--|peripheral| {"result": "AAAAAA33","status": 0}
14:37:17.772   INFO   <--|peripheral| retcode: 0
14:37:17.773   INFO   -->|peripheral| gattServer write 14 00112233
14:37:17.779   INFO   <--|peripheral| {"status": 0}
14:37:17.780   INFO   <--|peripheral| retcode: 0
14:37:17.780   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:17.904   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "00112233"}}
14:37:17.905   INFO   <--|central| retcode: 0
14:37:17.906   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AAAAAAAAAA
14:37:18.035   INFO   <--|central| {"status": -1,"error": "BLE_ERROR_INVALID_PARAM"}
14:37:18.037   INFO   <--|central| retcode: -1
14:37:18.038   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:18.183   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "00112233"}}
14:37:18.184   INFO   <--|central| retcode: 0
14:37:18.185   INFO   -->|peripheral| gattServer read 14
14:37:18.191   INFO   <--|peripheral| {"result": "00112233","status": 0}
14:37:18.192   INFO   <--|peripheral| retcode: 0
14:37:18.192   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AAAAAAAAAAAA
14:37:18.315   INFO   <--|central| {"status": -1,"error": "BLE_ERROR_INVALID_PARAM"}
14:37:18.316   INFO   <--|central| retcode: -1
14:37:18.317   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:18.463   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "00112233"}}
14:37:18.464   INFO   <--|central| retcode: 0
14:37:18.465   INFO   -->|peripheral| gattServer read 14
14:37:18.471   INFO   <--|peripheral| {"result": "00112233","status": 0}
14:37:18.472   INFO   <--|peripheral| retcode: 0
14:37:18.473   INFO   -->|central| gattClient writeCharacteristicDescriptor 1 14 AAAAAAAAAAAAAA
14:37:18.596   INFO   <--|central| {"status": -1,"error": "BLE_ERROR_INVALID_PARAM"}
14:37:18.598   INFO   <--|central| retcode: -1
14:37:18.599   INFO   -->|central| gattClient readCharacteristicDescriptor 1 14
14:37:18.743   INFO   <--|central| {"status": 0,"result": {"connection_handle": 1,"attribute_handle": 14,"offset": 0,"status": "BLE_ERROR_NONE","length": 4,"data": "00112233"}}
14:37:18.744   INFO   <--|central| retcode: 0
14:37:18.745   INFO   -->|peripheral| gattServer read 14
14:37:18.751   INFO   <--|peripheral| {"result": "00112233","status": 0}
14:37:18.753   INFO   <--|peripheral| retcode: 0
PASSED                                                                   [100%]

@ciarmcom
Copy link
Member

@paul-szczepanek-arm, thank you for your changes.
@ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested review from a team July 13, 2020 15:00
Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot added needs: CI and removed needs: review labels Jul 13, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 16, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Jul 16, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

@adbridge adbridge merged commit d14454b into ARMmbed:master Jul 17, 2020
@mergify mergify bot added the release version missing When PR does not contain release version, bot should label it and we fix it afterwards label Jul 17, 2020
@mergify
Copy link

mergify bot commented Jul 17, 2020

This PR does not contain release version label after merging.

@adbridge adbridge added release-type: patch Indentifies a PR as containing just a patch and removed Manually Patch release version missing When PR does not contain release version, bot should label it and we fix it afterwards labels Jul 17, 2020
@mbedmain mbedmain added release-version: 6.2.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants