Jira 794 Inconsistent writeInt() result with different Peripherals, g…

[SidLeung]

Root cause:

• The function, BLECharacteristicImp::write(), did not check the
  characteristic property to determine if the caller requires
  a response. It just treats all calls do not require a
  response.

Modifications:

1. libraries/CurieBLE/src/internal/BLECallbacks.cpp:
   • Added fake write response CB for write with no response call.
2. libraries/CurieBLE/src/internal/BLECallbacks.h:
   • Function definition added.
3. libraries/CurieBLE/src/internal/BLECharacteristicImp.cpp:
   • In function write(), check the characteristic property to
     determine whether the call requires a response.

[SidLeung]

@bigdinotech @eriknyquist , please review the code change. @noelpaz @russmcinnis , this PR originates from Flex PR #400. Please sign off if you use that PR for system verification.

[bigdinotech]

+1 but needs to be tested by someone else

[eriknyquist]

@SidLeung please fill in "reviewers" field in future... makes it much easier to track all the reviews :)

[eriknyquist]

Why the empty function? Let's delete it, if not need, or add a comment explaining WHY there is an empty function (e.g. a TODO or something)

[SidLeung]

I don't like that empty function neither. Let me check it out more. It should be made better.

[sgbihu]

Please see those two functions _bt_gatt_write and bt_gatt_write in the gatt.c. This try to schedule the write request with response.

[eriknyquist]

The code looks fine. I'd just like an explanation for the empty function

[SidLeung]

@sgbihu ,

I would suggest, at least for the Deneb release, the CB function, ble_on_write_no_rsp_complete, should not be left empty. I propose, based on the same logic as read, this routine should reset a flag that indicates the stack is in writing (eg. bool _inWriting). For write that requires no response, no checking of the _inWriting flag prior to the write() call. Otherwise, check for clearing of _inWriting flag before calling write().

[sgbihu]

This blank function want stack can send out write with response. If we are not define a function, the stack always schedule the write without command.

[SidLeung]

@sgbihu

Let me capture our discussion on how to make use of this call back function, ble_on_write_no_rsp_complete, here:

1. The options in the Characteristic determine the corresponding "write" command to be sent to a Peripheral. One is to perform a write without requiring a handshake response back from the Peripheral. The other command requires a handshake response from the Peripheral to complete the transaction.
2. The function, ble_on_write_no_rsp_complete, is a call back function for the Central to process the handshake response back from the Peripheral.
3. This mechanism determines the blocking scheme for a write operation,
   • Write without the handshake response from a Peripheral cannot be a blocking call. The transaction is complete once the stack generates the command.
   • Write with handshake synchronization with a Peripheral is always a blocking call. The stack refuses to process another write request until the previous one is acknowledged by the Peripheral.
4. Thus, ble_on_write_no_rsp_complete, can be the gatekeeper to keep track of response for the write with response transaction. Need to take into the account of multiple connections as the Central, one flag per connection(?).
5. Need to determine if the BLE stack has implemented timeout waiting on a response from a Peripheral. Otherwise, have to implement timeout at the Characteristic class.

[sgbihu]

Updated the code. Please rebase it.

[SidLeung]

For a Central, a write with response from the Peripheral is now made as a blocking call. The Characteristic is used to determine if a write operation require a full handshake.

@bigdinotech @eriknyquist , please review the addition code change.

@noelpaz @russmcinnis , please perform system verification with the change. Unit testing has been performed.

[noelpaz]

We are currently running tests

[noelpaz]

BAT test -- no impact on corelib all test passed.
Verified transaction in analyzer and saw that if Characteristic property is BLEWriteWithoutResponse then it is a direct command. If the characteristic property is BLEWrite which implies a response then you will see a Write Response Packet. Please see attached image:

[noelpaz]

QA team has verified the fix and also verified that it has not broken anything by running BAT

[SidLeung]

@yashaswini-hanji Would you please emerge this PR to main trunk?

[yashaswini-hanji]

Changes merged.