From acad65477065c5fad94feefabefb1fb6988d796c Mon Sep 17 00:00:00 2001 From: Austin Foxley Date: Wed, 27 Nov 2024 06:38:31 +0000 Subject: [PATCH] pw_bluetooth_proxy: Fix up checks in RfcommChannel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanups in logs, fix check conditions brought up in review. Change-Id: I7c590191969f97320b977aa288d8e9af741de185 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/250995 Pigweed-Auto-Submit: Austin Foxley Reviewed-by: David Rees Presubmit-Verified: CQ Bot Account Docs-Not-Needed: Ben Lawson Lint: Lint 🤖 Commit-Queue: Auto-Submit --- .../public/pw_bluetooth_proxy/rfcomm_channel.h | 2 +- pw_bluetooth_proxy/rfcomm_channel.cc | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h index 548e18dbc..e87242aee 100644 --- a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h +++ b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h @@ -108,7 +108,7 @@ class RfcommChannel final : public L2capWriteChannel, public L2capReadChannel { Config tx_config() const { return tx_config_; } private: - static constexpr uint8_t kMinRxCredits = 1; + static constexpr uint8_t kMinRxCredits = 2; RfcommChannel(L2capChannelManager& l2cap_channel_manager, uint16_t connection_handle, diff --git a/pw_bluetooth_proxy/rfcomm_channel.cc b/pw_bluetooth_proxy/rfcomm_channel.cc index 84fd28beb..22f17e114 100644 --- a/pw_bluetooth_proxy/rfcomm_channel.cc +++ b/pw_bluetooth_proxy/rfcomm_channel.cc @@ -62,6 +62,7 @@ pw::Status RfcommChannel::Write(pw::span payload) { length_extended_size + kCreditsFieldSize + payload.size(); + // TODO: https://pwbug.dev/365179076 - Support fragmentation. pw::Result h4_result = PopulateTxL2capPacket(frame_size); if (!h4_result.ok()) { return h4_result.status(); @@ -112,6 +113,7 @@ pw::Status RfcommChannel::Write(pw::span payload) { if (rfcomm.information().SizeInBytes() < payload.size()) { return Status::ResourceExhausted(); } + PW_CHECK(rfcomm.information().SizeInBytes() == payload.size()); std::memcpy(rfcomm.information().BackingStorage().data(), payload.data(), payload.size()); @@ -148,7 +150,8 @@ Result RfcommChannel::Create( uint8_t channel_number, pw::Function payload)>&& receive_fn) { if (!L2capWriteChannel::AreValidParameters(connection_handle, - tx_config.cid)) { + tx_config.cid) || + !L2capReadChannel::AreValidParameters(connection_handle, rx_config.cid)) { return Status::InvalidArgument(); } @@ -170,10 +173,9 @@ bool RfcommChannel::HandlePduFromController(pw::span l2cap_pdu) { MakeEmbossView(l2cap_pdu); if (!bframe_view.ok()) { PW_LOG_ERROR( - "(CID 0x%X) Buffer is too small for L2CAP B-frame. So stopping channel " - "& reporting it needs to be closed.", + "(CID 0x%X) Buffer is too small for L2CAP B-frame, passing on to host.", local_cid()); - return true; + return false; } Result rfcomm_view = @@ -232,7 +234,7 @@ bool RfcommChannel::HandlePduFromController(pw::span l2cap_pdu) { } else { --rx_credits_; } - rx_needs_refill = rx_credits_ <= kMinRxCredits; + rx_needs_refill = rx_credits_ < kMinRxCredits; } if (rx_needs_refill) { @@ -276,7 +278,7 @@ RfcommChannel::RfcommChannel( void RfcommChannel::OnFragmentedPduReceived() { PW_LOG_ERROR( "(CID 0x%X) Fragmented L2CAP frame received (which is not yet " - "supported). Stopping channel.", + "supported).", local_cid()); }