From 22ec962a552463bcf18c54b42d548ba8b297a23e Mon Sep 17 00:00:00 2001 From: Ali Saeed Date: Fri, 6 Dec 2024 23:23:07 +0000 Subject: [PATCH] pw_bluetooth_proxy: Write Identifier in signaling command headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 380076024 Change-Id: Ib95f17ae01cb628ad0e999076677fd1790126af0 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/252952 Commit-Queue: Ali Saeed Lint: Lint 🤖 Reviewed-by: David Rees Docs-Not-Needed: Ali Saeed Reviewed-by: Ben Lawson --- pw_bluetooth_proxy/l2cap_signaling_channel.cc | 9 +++++++++ pw_bluetooth_proxy/proxy_host_test.cc | 9 +++++++++ .../internal/l2cap_signaling_channel.h | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/pw_bluetooth_proxy/l2cap_signaling_channel.cc b/pw_bluetooth_proxy/l2cap_signaling_channel.cc index fd7a8546b..0293a5aa0 100644 --- a/pw_bluetooth_proxy/l2cap_signaling_channel.cc +++ b/pw_bluetooth_proxy/l2cap_signaling_channel.cc @@ -278,6 +278,7 @@ Status L2capSignalingChannel::SendFlowControlCreditInd(uint16_t cid, cframe.payload().SizeInBytes()); ind.command_header().code().Write( emboss::L2capSignalingPacketCode::FLOW_CONTROL_CREDIT_IND); + ind.command_header().identifier().Write(GetNextIdentifierAndIncrement()); ind.command_header().data_length().Write( emboss::L2capFlowControlCreditInd::IntrinsicSizeInBytes() - emboss::L2capSignalingCommandHeader::IntrinsicSizeInBytes()); @@ -287,4 +288,12 @@ Status L2capSignalingChannel::SendFlowControlCreditInd(uint16_t cid, return QueuePacket(std::move(h4_packet)); } +uint8_t L2capSignalingChannel::GetNextIdentifierAndIncrement() { + if (next_identifier_ == UINT8_MAX) { + next_identifier_ = 1; + return UINT8_MAX; + } + return next_identifier_++; +} + } // namespace pw::bluetooth::proxy diff --git a/pw_bluetooth_proxy/proxy_host_test.cc b/pw_bluetooth_proxy/proxy_host_test.cc index fc65674d8..f9a79d4c2 100644 --- a/pw_bluetooth_proxy/proxy_host_test.cc +++ b/pw_bluetooth_proxy/proxy_host_test.cc @@ -3996,6 +3996,15 @@ TEST(L2capSignalingTest, RxAdditionalCreditsSentOnL2capCocAcquisition) { emboss::MakeL2capFlowControlCreditIndView( cframe.payload().BackingStorage().data(), cframe.payload().SizeInBytes()); + EXPECT_EQ(ind.command_header().code().Read(), + emboss::L2capSignalingPacketCode::FLOW_CONTROL_CREDIT_IND); + // TODO: https://pwbug.dev/382553099 - Test to ensure we are properly + // incrementing Identifier when sending multiple signaling packets. + EXPECT_EQ(ind.command_header().identifier().Read(), 1); + EXPECT_EQ( + ind.command_header().data_length().Read(), + emboss::L2capFlowControlCreditInd::IntrinsicSizeInBytes() - + emboss::L2capSignalingCommandHeader::IntrinsicSizeInBytes()); EXPECT_EQ(ind.cid().Read(), capture.local_cid); EXPECT_EQ(ind.credits().Read(), capture.credits); }); diff --git a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_signaling_channel.h b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_signaling_channel.h index cc3f1046a..64bfabde5 100644 --- a/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_signaling_channel.h +++ b/pw_bluetooth_proxy/public/pw_bluetooth_proxy/internal/l2cap_signaling_channel.h @@ -14,7 +14,6 @@ #pragma once -#include "pw_bluetooth/hci_data.emb.h" #include "pw_bluetooth/l2cap_frames.emb.h" #include "pw_bluetooth_proxy/basic_l2cap_channel.h" #include "pw_bluetooth_proxy/l2cap_status_delegate.h" @@ -89,6 +88,10 @@ class L2capSignalingChannel : public BasicL2capChannel { bool HandlePduFromHost(pw::span cframe) override; + // Get the next Identifier value that should be written to a signaling + // command and increment the Identifier. + uint8_t GetNextIdentifierAndIncrement(); + L2capChannelManager& l2cap_channel_manager_; private: @@ -107,6 +110,16 @@ class L2capSignalingChannel : public BasicL2capChannel { PW_GUARDED_BY(mutex_){}; sync::Mutex mutex_; + + // Core Spec v6.0 Vol 3, Part A, Section 4: "The Identifier field is one octet + // long and matches responses with requests. The requesting device sets this + // field and the responding device uses the same value in its response. Within + // each signaling channel a different Identifier shall be used for each + // successive command. Following the original transmission of an Identifier in + // a command, the Identifier may be recycled if all other Identifiers have + // subsequently been used." + // TODO: https://pwbug.dev/382553099 - Synchronize this value with AP host. + uint8_t next_identifier_ = 1; }; } // namespace pw::bluetooth::proxy