rymanluk commented on code in PR #1271:
URL: https://github.com/apache/mynewt-nimble/pull/1271#discussion_r930146040
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -67,9 +67,9 @@ ble_l2cap_coc_srv_alloc(void)
int
ble_l2cap_coc_create_server(uint16_t psm, uint16_t mtu,
- ble_l2cap_event_fn *cb, void *cb_arg)
+ ble_l2cap_event_fn *cb, void *cb_arg)
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -35,7 +35,7 @@ static struct ble_l2cap_coc_srv_list ble_l2cap_coc_srvs;
static os_membuf_t ble_l2cap_coc_srv_mem[
OS_MEMPOOL_SIZE(MYNEWT_VAL(BLE_L2CAP_COC_MAX_NUM),
- sizeof (struct ble_l2cap_coc_srv))
+ sizeof(struct ble_l2cap_coc_srv))
Review Comment:
Can we make all those minor coding style changes as a separate patch ?
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -288,14 +298,16 @@ ble_l2cap_coc_rx_fn(struct ble_l2cap_chan *chan)
ble_l2cap_sig_le_credits(chan->conn_handle, chan->scid, rx->credits);
}
- BLE_HS_LOG(DEBUG, "Received partial sdu_len=%d, credits left=%d\n",
- OS_MBUF_PKTLEN(rx->sdu), rx->credits);
+ BLE_HS_LOG(DEBUG,
+ "Received partial sdu_len=%d, credits left=%d,
current_sdu=%d\n",
+ OS_MBUF_PKTLEN(rx_sdu), rx->credits, chan->coc_rx.current_sdu);
return 0;
}
void
-ble_l2cap_coc_set_new_mtu_mps(struct ble_l2cap_chan *chan, uint16_t mtu,
uint16_t mps)
+ble_l2cap_coc_set_new_mtu_mps(struct ble_l2cap_chan *chan, uint16_t mtu,
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -218,12 +225,13 @@ ble_l2cap_coc_rx_fn(struct ble_l2cap_chan *chan)
return BLE_HS_EBADDATA;
}
- BLE_HS_LOG(DEBUG, "sdu_len=%d, received LE frame=%d, credits=%d\n",
- sdu_len, om_total, rx->credits);
+ BLE_HS_LOG(DEBUG,
+ "sdu_len=%d, received LE frame=%d, credits=%d,
current_sdu=%d\n",
+ sdu_len, om_total, rx->credits, chan->coc_rx.current_sdu);
- os_mbuf_adj(*om , BLE_L2CAP_SDU_SIZE);
+ os_mbuf_adj(*om, BLE_L2CAP_SDU_SIZE);
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -113,7 +113,7 @@ ble_l2cap_get_first_available_bit(uint32_t *cid_mask)
* a) If bit == 0 means all the bits are used
* b) this function returns 1 + index
*/
- bit = __builtin_ffs(~(unsigned int)(cid_mask[i]));
+ bit = __builtin_ffs(~(unsigned int) (cid_mask[i]));
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -148,8 +148,8 @@ ble_l2cap_coc_srv_find(uint16_t psm)
srv = NULL;
STAILQ_FOREACH(cur, &ble_l2cap_coc_srvs, next) {
if (cur->psm == psm) {
- srv = cur;
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -67,9 +67,9 @@ ble_l2cap_coc_srv_alloc(void)
int
ble_l2cap_coc_create_server(uint16_t psm, uint16_t mtu,
- ble_l2cap_event_fn *cb, void *cb_arg)
+ ble_l2cap_event_fn *cb, void *cb_arg)
{
- struct ble_l2cap_coc_srv * srv;
+ struct ble_l2cap_coc_srv *srv;
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -362,7 +379,7 @@ ble_l2cap_coc_create_srv_chan(struct ble_hs_conn *conn,
uint16_t psm,
static void
ble_l2cap_event_coc_disconnected(struct ble_l2cap_chan *chan)
{
- struct ble_l2cap_event event = { };
+ struct ble_l2cap_event event = {};
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -377,11 +394,12 @@ ble_l2cap_event_coc_disconnected(struct ble_l2cap_chan
*chan)
}
void
-ble_l2cap_coc_cleanup_chan(struct ble_hs_conn *conn, struct ble_l2cap_chan
*chan)
+ble_l2cap_coc_cleanup_chan(struct ble_hs_conn *conn,
Review Comment:
really needed? If so "separate patch"
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -377,11 +394,12 @@ ble_l2cap_event_coc_disconnected(struct ble_l2cap_chan
*chan)
}
void
-ble_l2cap_coc_cleanup_chan(struct ble_hs_conn *conn, struct ble_l2cap_chan
*chan)
+ble_l2cap_coc_cleanup_chan(struct ble_hs_conn *conn,
+ struct ble_l2cap_chan *chan)
{
/* PSM 0 is used for fixed channels. */
if (chan->psm == 0) {
- return;
+ return;
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -391,14 +409,16 @@ ble_l2cap_coc_cleanup_chan(struct ble_hs_conn *conn,
struct ble_l2cap_chan *chan
chan->scid - BLE_L2CAP_COC_CID_START);
}
- os_mbuf_free_chain(chan->coc_rx.sdu);
- os_mbuf_free_chain(chan->coc_tx.sdu);
+ for (int i = 0; i < BLE_L2CAP_SDU_BUFF_CNT; i++) {
+ os_mbuf_free_chain(chan->coc_rx.sdus[i]);
+ }
+ os_mbuf_free_chain(chan->coc_tx.sdus[0]);
}
static void
ble_l2cap_event_coc_unstalled(struct ble_l2cap_chan *chan, int status)
{
- struct ble_l2cap_event event = { };
+ struct ble_l2cap_event event = {};
Review Comment:
separate patch
##########
nimble/host/src/ble_l2cap_coc.c:
##########
@@ -324,7 +336,12 @@ ble_l2cap_coc_chan_alloc(struct ble_hs_conn *conn,
uint16_t psm, uint16_t mtu,
chan->my_coc_mps = MYNEWT_VAL(BLE_L2CAP_COC_MPS);
chan->rx_fn = ble_l2cap_coc_rx_fn;
chan->coc_rx.mtu = mtu;
- chan->coc_rx.sdu = sdu_rx;
+ chan->coc_rx.sdus[0] = sdu_rx;
+ for (int i = 1; i < BLE_L2CAP_SDU_BUFF_CNT; i++) {
+ chan->coc_rx.sdus[i] = NULL;
+ }
+ chan->coc_rx.current_sdu = 0;
+ chan->coc_rx.next_sdu = 0;
Review Comment:
Could you put comment in the code what 'next sdu' and 'current sdu' are for?
btw looking into line 607 I think it should be set to 1 here, or ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
