nimble/audio: Modify BASS control point write access. #1833
Conversation
|
|
||
| uint8_t opcode = ctxt->om->om_data[0]; | ||
| os_mbuf_copydata(ctxt->om, 0, mbuf_len, data); |
There was a problem hiding this comment.
are we sure we want to put those data on the stack?
There was a problem hiding this comment.
Previous implementation produced some problems regarding passing data into specific handlers, as not all of the data was present - after discussing this with @sjanc we've decided that a good solution would be to use os_mbuf for passing the data that write access function handles.
Edit: The incorrect amount of data was a result of chained mbufs.
There was a problem hiding this comment.
Could you please improve the commit message, I'm not sure I understand what this change supposed to fix.
But I see what it breaks unfortunately. The data you pass to the handler include the opcode (as you pass &data[0], not &data[1]), while the handles assume the 0 parameter won't be opcode
| uint16_t mbuf_len = OS_MBUF_PKTLEN(ctxt->om); | ||
| uint8_t opcode; | ||
| uint8_t data[MYNEWT_VAL(BLE_SVC_AUDIO_BASS_METADATA_MAX_SZ)]; |
There was a problem hiding this comment.
please follow the coding style - code is not aligned
szymon-czapracki
force-pushed
the
bass_ctrlp_write_access
branch
6 times, most recently
from
3f3e343
to
c9d531e
Compare
| @@ -633,7 +636,12 @@ ble_svc_audio_bass_ctrl_point_write_access(struct ble_gatt_access_ctxt *ctxt, ui | |||
| return BLE_ATT_ERR_WRITE_REQ_REJECTED; | |||
| } | |||
|
|
|||
| return handler->handler_cb(&ctxt->om->om_data[1], ctxt->om->om_len - 1, conn_handle); | |||
| om = os_mbuf_pullup(ctxt->om, mbuf_len); | |||
There was a problem hiding this comment.
Disregard om, use ctx->om
There was a problem hiding this comment.
Done @rymanluk LGTY?
Modify BASS control point write access in a way that it would use flattened mbuf to pass data to handler. Previous solution was prone to errors due to chained mbufs. Fix handler check from simple bool comparison to ensuring that it's not NULL. Add opcode variable to hold proper operation id in write access. This is helpful during debugging operations.
szymon-czapracki
force-pushed
the
bass_ctrlp_write_access
branch
from
c9d531e
to
c6e294a
Compare
Modify BASS control point write access in a way that it would use proper mbuf for holding incoming data.
Fix handler check from simple bool comparison to ensuring that it's not NULL.