Skip to content

Jira 795, read() is blocking selectable, git 383 #418

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.

Sign up for GitHub

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
merged 1 commit into from
Feb 23, 2017
Merged

Jira 795, read() is blocking selectable, git 383 #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libraries/CurieBLE/src/BLECharacteristic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,14 @@ bool BLECharacteristic::canUnsubscribe()
return retVar;
}

bool BLECharacteristic::read()
bool BLECharacteristic::read(bool blocked)
{
bool retVar = false;
BLECharacteristicImp *characteristicImp = getImplementation();

if (NULL != characteristicImp)
{
retVar = characteristicImp->read();
retVar = characteristicImp->read(blocked);
}
return retVar;
}
Expand Down
3 changes: 2 additions & 1 deletion libraries/CurieBLE/src/BLECharacteristic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,9 @@ class BLECharacteristic: public BLEAttributeWithValue
* @return bool true - Success, false - Failed
*
* @note Only for GATT client. Schedule read request to the GATT server
* Arduino requests to have read, by default, be blocking.
*/
virtual bool read();
virtual bool read(bool blocked = true);

/**
* @brief Write the charcteristic value
Expand Down
2 changes: 1 addition & 1 deletion libraries/CurieBLE/src/internal/BLECallbacks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ uint8_t profile_read_rsp_process(bt_conn_t *conn,
const void *data,
uint16_t length)
{
if (NULL == data)
if (NULL == data && 0 != length)
{
return BT_GATT_ITER_STOP;
}
Expand Down
18 changes: 15 additions & 3 deletions libraries/CurieBLE/src/internal/BLECharacteristicImp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,10 @@ bt_uuid_t* BLECharacteristicImp::getClientCharacteristicConfigUuid(void)
return (bt_uuid_t*) &_gatt_ccc_uuid;
}

bool BLECharacteristicImp::read()
bool BLECharacteristicImp::read(bool blocked)
{
int retval = 0;
bool ret_bool = false;
bt_conn_t* conn = NULL;

if (true == BLEUtils::isLocalBLE(_ble_device))
Expand Down Expand Up @@ -631,12 +632,23 @@ bool BLECharacteristicImp::read()

// Send read request
retval = bt_gatt_read(conn, &_read_params);
bt_conn_unref(conn);
if (0 == retval)
{
_reading = true;
ret_bool = true;

// Block the call
if (blocked == true)
{
while (_reading == true && ret_bool)
{
delay(5);
ret_bool = _ble_device.connected();
}
}
}
return _reading;
bt_conn_unref(conn);
return ret_bool;
}

void BLECharacteristicImp::writeResponseReceived(struct bt_conn *conn,
Expand Down
10 changes: 5 additions & 5 deletions libraries/CurieBLE/src/internal/BLECharacteristicImp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,18 @@ class BLECharacteristicImp: public BLEAttribute{
/**
* @brief Schedule the read request to read the characteristic in peripheral
*
* @param[in] none
* @param[in] blocked Flag the call is blocked or un-blocked
*
* @return bool Indicate the success or error
*
* @note Only for central device
* @note Only for GATT client
* Default it is block call as per Arduino request
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

So this doesn't change any existing code, right? i.e. if I call read() with no arguments, it will block by default, yes?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct.

bool read();
bool read(bool blocked = true);

/**
* @brief Schedule the write request to update the characteristic in peripheral
*
* @param[in] peripheral The peripheral device that want to be updated
* @param[in] value New value to set, as a byte array. Data is stored in internal copy.
* @param[in] length Length, in bytes, of valid data in the array to write.
* Must not exceed maxLength set for this characteristic.
Expand Down Expand Up @@ -328,7 +328,7 @@ class BLECharacteristicImp: public BLEAttribute{
bt_gatt_subscribe_params_t _sub_params;
bool _subscribed;

bool _reading;
volatile bool _reading;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need to be volatile?

Copy link
Contributor

@sgbihu sgbihu Feb 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Please note last two changes in BLECharacteristicImp.cpp. It need to make compiler not optimize the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

So we typing it as volatile so that it compile? Is this change temporary then? What is the penalty as far as code size?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so why do you need to force optimization off? Who is accessing this variable out of normal execution flow? Describe the situation that will occur if _reading is not volatile.

Copy link
Contributor

Choose a reason for hiding this comment

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

When read response come back, the setValue will be called and clear the reading flag. Because the response is asynchronous. So the below code may optimized for the value was set true before call while.

_reading = true;

while(_reading)
{
......
}

static volatile bool _gattc_writing;
bt_gatt_read_params_t _read_params; // GATT read parameter

Expand Down