diff --git a/pw_bluetooth_proxy/l2cap_channel.cc b/pw_bluetooth_proxy/l2cap_channel.cc index 92f2148c1..87e9eda41 100644 --- a/pw_bluetooth_proxy/l2cap_channel.cc +++ b/pw_bluetooth_proxy/l2cap_channel.cc @@ -146,14 +146,32 @@ Status L2capChannel::QueuePacket(H4PacketWithH4&& packet) { return status; } +namespace { + +pw::span AsConstUint8Span(ConstByteSpan s) { + return {reinterpret_cast(s.data()), s.size_bytes()}; +} + +} // namespace + StatusWithMultiBuf L2capChannel::Write(multibuf::MultiBuf&& payload) { - if (!UsesPayloadQueue()) { - PW_LOG_ERROR( - "btproxy: Write(MultiBuf) called on class that only supports " - "Write(span)."); - return {Status::Unimplemented(), std::move(payload)}; + if (!payload.IsContiguous()) { + return {Status::InvalidArgument(), std::move(payload)}; + } + + if (state() != State::kRunning) { + return {Status::FailedPrecondition(), std::move(payload)}; } + PW_CHECK(UsesPayloadQueue()); + + return QueuePayload(std::move(payload)); +} + +// TODO: https://pwbug.dev/379337272 - Delete when all channels are +// transitioned to using payload queues. +StatusWithMultiBuf L2capChannel::WriteMultiBufAsSpan( + multibuf::MultiBuf&& payload) { if (!payload.IsContiguous()) { return {Status::InvalidArgument(), std::move(payload)}; } @@ -162,7 +180,17 @@ StatusWithMultiBuf L2capChannel::Write(multibuf::MultiBuf&& payload) { return {Status::FailedPrecondition(), std::move(payload)}; } - return QueuePayload(std::move(payload)); + PW_CHECK(!UsesPayloadQueue()); + + std::optional span = payload.ContiguousSpan(); + PW_CHECK(span.has_value()); + Status status = Write(AsConstUint8Span(span.value())); + + if (!status.ok()) { + return {status, std::move(payload)}; + } + + return {OkStatus(), std::nullopt}; } pw::Status L2capChannel::Write( diff --git a/pw_bluetooth_proxy/proxy_host_test.cc b/pw_bluetooth_proxy/proxy_host_test.cc index f78e03b1f..177419ee1 100644 --- a/pw_bluetooth_proxy/proxy_host_test.cc +++ b/pw_bluetooth_proxy/proxy_host_test.cc @@ -1458,7 +1458,8 @@ TEST_F(NumberOfCompletedPacketsTest, MultipleChannelsDifferentTransports) { RfcommChannel bredr_channel = BuildRfcomm(proxy, RfcommParameters{.handle = 0x456}); - EXPECT_EQ(bredr_channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + bredr_channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); // Send should succeed even though no LE credits available EXPECT_EQ(capture.sends_called, 2); @@ -1476,7 +1477,8 @@ TEST_F(NumberOfCompletedPacketsTest, MultipleChannelsDifferentTransports) { proxy, FlatMap({{{0x456, 1}}}))); // Write again - EXPECT_EQ(bredr_channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + bredr_channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); EXPECT_EQ(capture.sends_called, 4); } @@ -2171,7 +2173,8 @@ TEST_F(BasicL2capChannelTest, BasicWrite) { /*payload_from_controller_fn=*/nullptr, /*event_fn=*/nullptr)); - EXPECT_EQ(channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); EXPECT_EQ(capture.sends_called, 1); } @@ -2202,7 +2205,8 @@ TEST_F(BasicL2capChannelTest, ErrorOnWriteTooLarge) { /*payload_from_controller_fn=*/nullptr, /*event_fn=*/nullptr)); - EXPECT_EQ(channel.Write(span(hci_arr)), PW_STATUS_INVALID_ARGUMENT); + EXPECT_EQ(channel.Write(MultiBufFromSpan(pw::span(hci_arr))).status, + PW_STATUS_INVALID_ARGUMENT); } TEST_F(BasicL2capChannelTest, CannotCreateChannelWithInvalidArgs) { diff --git a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/basic_l2cap_channel.h b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/basic_l2cap_channel.h index 4ac882047..135345907 100644 --- a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/basic_l2cap_channel.h +++ b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/basic_l2cap_channel.h @@ -39,7 +39,17 @@ class BasicL2capChannel : public L2capChannel { BasicL2capChannel& operator=(BasicL2capChannel&& other) = default; ~BasicL2capChannel() override; - pw::Status Write(pw::span payload) override; + // @deprecated + // TODO: https://pwbug.dev/379337272 - Delete this once all downstreams + // have transitioned to Write(MultiBuf) for this channel type. + Status Write(pw::span payload) override; + + // Also allow Write(MultiBuf) during transition from Write(span). + // TODO: https://pwbug.dev/379337272 - Can delete once Write(span) above is + // deleted. + StatusWithMultiBuf Write(multibuf::MultiBuf&& payload) override { + return WriteMultiBufAsSpan(std::move(payload)); + } protected: explicit BasicL2capChannel( diff --git a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_channel.h b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_channel.h index 54c2516b7..8815583f0 100644 --- a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_channel.h +++ b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_channel.h @@ -236,6 +236,17 @@ class L2capChannel : public IntrusiveForwardList::Item { // Tx (protected) //---------------- + // Sends a MultiBuf payload to the remote peer using Write(span). + // + // The contents of the MultiBuf are copied during the call and the MultiBuf + // is destroyed. + // + // Used by subclasses during the transition from PDU send_queue_ rather to + // Write(MultiBuf) and payload queues. + // TODO: https://pwbug.dev/379337272 - Delete when all channels are + // transitioned to using payload queues. + StatusWithMultiBuf WriteMultiBufAsSpan(multibuf::MultiBuf&& payload); + // Queue a client `buf` for sending and `ReportPacketsMayBeReadyToSend()`. // Must be a contiguous MultiBuf. // 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 eea5540d1..7d224dea8 100644 --- a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h +++ b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/rfcomm_channel.h @@ -87,8 +87,18 @@ class RfcommChannel final : public L2capChannel { Function payload)>&& payload_from_controller_fn, Function&& event_fn); + // @deprecated + // TODO: https://pwbug.dev/379337272 - Delete this once all downstreams + // have transitioned to Write(MultiBuf) for this channel type. Status Write(pw::span payload) override; + // Also allow Write(MultiBuf) during transition from Write(span). + // TODO: https://pwbug.dev/379337272 - Can delete once Write(span) above is + // deleted. + StatusWithMultiBuf Write(multibuf::MultiBuf&& payload) override { + return WriteMultiBufAsSpan(std::move(payload)); + } + Config rx_config() const { return rx_config_; } Config tx_config() const { return tx_config_; } diff --git a/pw_bluetooth_proxy/pw_bluetooth_proxy_private/test_utils.h b/pw_bluetooth_proxy/pw_bluetooth_proxy_private/test_utils.h index 50d4708ec..fa9c6f465 100644 --- a/pw_bluetooth_proxy/pw_bluetooth_proxy_private/test_utils.h +++ b/pw_bluetooth_proxy/pw_bluetooth_proxy_private/test_utils.h @@ -278,7 +278,7 @@ class ProxyHostTest : public testing::Test { private: // MultiBuf allocator for creating objects to pass to the system under // test (e.g. creating test packets to send to proxy host). - pw::multibuf::test::SimpleAllocatorForTest test_multibuf_allocator_{}; diff --git a/pw_bluetooth_proxy/rfcomm_channel.cc b/pw_bluetooth_proxy/rfcomm_channel.cc index 606c652d1..1732c7bb8 100644 --- a/pw_bluetooth_proxy/rfcomm_channel.cc +++ b/pw_bluetooth_proxy/rfcomm_channel.cc @@ -240,7 +240,8 @@ bool RfcommChannel::DoHandlePduFromController(pw::span l2cap_pdu) { if (rx_needs_refill) { // Send credit update with empty payload to refresh remote credit count. - if (const auto status = Write({}); !status.ok()) { + if (const auto status = Write(pw::multibuf::MultiBuf{}).status; + !status.ok()) { PW_LOG_ERROR("Failed to send RFCOMM credits"); } } diff --git a/pw_bluetooth_proxy/rfcomm_test.cc b/pw_bluetooth_proxy/rfcomm_test.cc index 33ab08a52..6b04fd9f7 100644 --- a/pw_bluetooth_proxy/rfcomm_test.cc +++ b/pw_bluetooth_proxy/rfcomm_test.cc @@ -190,7 +190,8 @@ TEST_F(RfcommWriteTest, BasicWrite) { .credits = 10, }}; RfcommChannel channel = BuildRfcomm(proxy, params); - EXPECT_EQ(channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); EXPECT_EQ(capture.sends_called, 1); } @@ -302,7 +303,8 @@ TEST_F(RfcommWriteTest, ExtendedWrite) { .credits = 10, }}; RfcommChannel channel = BuildRfcomm(proxy, params); - EXPECT_EQ(channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); EXPECT_EQ(capture.sends_called, 1); } @@ -346,7 +348,8 @@ TEST_F(RfcommWriteTest, WriteFlowControl) { // Writes while queue has space will return Ok. No RFCOMM credits yet though // so no sends complete. - EXPECT_EQ(channel.Write(capture.payload), PW_STATUS_OK); + PW_TEST_EXPECT_OK( + channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status); EXPECT_EQ(capture.sends_called, 0); EXPECT_EQ(capture.queue_unblocked, 0); @@ -360,7 +363,8 @@ TEST_F(RfcommWriteTest, WriteFlowControl) { // Now fill up queue uint16_t queued = 0; while (true) { - if (const auto status = channel.Write(capture.payload); + if (const auto status = + channel.Write(MultiBufFromSpan(pw::span(capture.payload))).status; status == Status::Unavailable()) { break; }