Skip to content

Commit

Permalink
pw_bluetooth_proxy: Clarify naming on L2capReadChannel payload functions
Browse files Browse the repository at this point in the history
Change-Id: Ia489cbc9d676b4cd31b0cce8ab6adef6c34dd1f0
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/251112
Commit-Queue: Auto-Submit <[email protected]>
Docs-Not-Needed: Ben Lawson <[email protected]>
Pigweed-Auto-Submit: Austin Foxley <[email protected]>
Lint: Lint 🤖 <[email protected]>
Reviewed-by: Ben Lawson <[email protected]>
  • Loading branch information
afoxley authored and CQ Bot Account committed Nov 27, 2024
1 parent 91d9808 commit 027897c
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 53 deletions.
14 changes: 8 additions & 6 deletions pw_bluetooth_proxy/basic_l2cap_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ pw::Result<BasicL2capChannel> BasicL2capChannel::Create(
L2capChannelManager& l2cap_channel_manager,
uint16_t connection_handle,
uint16_t local_cid,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn) {
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn) {
if (!L2capReadChannel::AreValidParameters(connection_handle, local_cid)) {
return pw::Status::InvalidArgument();
}
Expand All @@ -33,7 +34,7 @@ pw::Result<BasicL2capChannel> BasicL2capChannel::Create(
/*l2cap_channel_manager=*/l2cap_channel_manager,
/*connection_handle=*/connection_handle,
/*local_cid=*/local_cid,
/*controller_receive_fn=*/std::move(controller_receive_fn));
/*payload_from_controller_fn=*/std::move(payload_from_controller_fn));
}

BasicL2capChannel& BasicL2capChannel::operator=(BasicL2capChannel&& other) {
Expand All @@ -45,9 +46,9 @@ BasicL2capChannel::BasicL2capChannel(
L2capChannelManager& l2cap_channel_manager,
uint16_t connection_handle,
uint16_t local_cid,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn)
pw::Function<void(pw::span<uint8_t> payload)>&& payload_from_controller_fn)
: L2capReadChannel(l2cap_channel_manager,
std::move(controller_receive_fn),
std::move(payload_from_controller_fn),
connection_handle,
local_cid) {}

Expand All @@ -60,8 +61,9 @@ bool BasicL2capChannel::HandlePduFromController(pw::span<uint8_t> bframe) {
PW_LOG_ERROR("(CID: 0x%X) Received invalid B-frame. So will drop.",
local_cid());
} else {
CallControllerReceiveFn(span(bframe_view->payload().BackingStorage().data(),
bframe_view->payload().SizeInBytes()));
SendPayloadFromControllerToClient(
span(bframe_view->payload().BackingStorage().data(),
bframe_view->payload().SizeInBytes()));
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion pw_bluetooth_proxy/l2cap_coc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ bool L2capCoc::HandlePduFromController(pw::span<uint8_t> kframe) {
return true;
}

CallControllerReceiveFn(pw::span(
SendPayloadFromControllerToClient(pw::span(
const_cast<uint8_t*>(kframe_view->payload().BackingStorage().data()),
kframe_view->payload_size().Read()));
return true;
Expand Down
8 changes: 4 additions & 4 deletions pw_bluetooth_proxy/l2cap_read_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace pw::bluetooth::proxy {
L2capReadChannel::L2capReadChannel(L2capReadChannel&& other)
: connection_handle_(other.connection_handle()),
local_cid_(other.local_cid()),
controller_receive_fn_(std::move(other.controller_receive_fn_)),
payload_from_controller_fn_(std::move(other.payload_from_controller_fn_)),
l2cap_channel_manager_(other.l2cap_channel_manager_) {
l2cap_channel_manager_.ReleaseReadChannel(other);
l2cap_channel_manager_.RegisterReadChannel(*this);
Expand All @@ -35,7 +35,7 @@ L2capReadChannel& L2capReadChannel::operator=(L2capReadChannel&& other) {
"(still registered with L2capChannelManager).");
connection_handle_ = other.connection_handle();
local_cid_ = other.local_cid();
controller_receive_fn_ = std::move(other.controller_receive_fn_);
payload_from_controller_fn_ = std::move(other.payload_from_controller_fn_);
l2cap_channel_manager_.ReleaseReadChannel(other);
l2cap_channel_manager_.RegisterReadChannel(*this);
}
Expand All @@ -54,12 +54,12 @@ void L2capReadChannel::OnFragmentedPduReceived() {

L2capReadChannel::L2capReadChannel(
L2capChannelManager& l2cap_channel_manager,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn,
pw::Function<void(pw::span<uint8_t> payload)>&& payload_from_controller_fn,
uint16_t connection_handle,
uint16_t local_cid)
: connection_handle_(connection_handle),
local_cid_(local_cid),
controller_receive_fn_(std::move(controller_receive_fn)),
payload_from_controller_fn_(std::move(payload_from_controller_fn)),
l2cap_channel_manager_(l2cap_channel_manager) {
l2cap_channel_manager_.RegisterReadChannel(*this);
}
Expand Down
2 changes: 1 addition & 1 deletion pw_bluetooth_proxy/l2cap_signaling_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ L2capSignalingChannel::L2capSignalingChannel(
: BasicL2capChannel(/*l2cap_channel_manager=*/l2cap_channel_manager,
/*connection_handle=*/connection_handle,
/*local_cid=*/local_cid,
/*controller_receive_fn=*/nullptr),
/*payload_from_controller_fn=*/nullptr),
l2cap_channel_manager_(l2cap_channel_manager) {}

L2capSignalingChannel& L2capSignalingChannel::operator=(
Expand Down
5 changes: 3 additions & 2 deletions pw_bluetooth_proxy/proxy_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,8 @@ pw::Result<BasicL2capChannel> ProxyHost::AcquireBasicL2capChannel(
uint16_t connection_handle,
uint16_t local_cid,
AclTransportType transport,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn) {
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn) {
Status status =
acl_data_channel_.CreateAclConnection(connection_handle, transport);
if (status.IsResourceExhausted()) {
Expand All @@ -402,7 +403,7 @@ pw::Result<BasicL2capChannel> ProxyHost::AcquireBasicL2capChannel(
/*l2cap_channel_manager=*/l2cap_channel_manager_,
/*connection_handle=*/connection_handle,
/*local_cid=*/local_cid,
/*controller_receive_fn=*/std::move(controller_receive_fn));
/*payload_from_controller_fn=*/std::move(payload_from_controller_fn));
}

pw::Status ProxyHost::SendGattNotify(uint16_t connection_handle,
Expand Down
32 changes: 17 additions & 15 deletions pw_bluetooth_proxy/proxy_host_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2183,22 +2183,24 @@ TEST(BasicL2capChannelTest, CannotCreateChannelWithInvalidArgs) {
std::move(send_to_host_fn), std::move(send_to_controller_fn), 0);

// Connection handle too large by 1.
EXPECT_EQ(proxy
.AcquireBasicL2capChannel(/*connection_handle=*/0x0FFF,
/*local_cid=*/0x123,
/*type=*/AclTransportType::kLe,
/*controller_receive_fn=*/nullptr)
.status(),
PW_STATUS_INVALID_ARGUMENT);
EXPECT_EQ(
proxy
.AcquireBasicL2capChannel(/*connection_handle=*/0x0FFF,
/*local_cid=*/0x123,
/*type=*/AclTransportType::kLe,
/*payload_from_controller_fn=*/nullptr)
.status(),
PW_STATUS_INVALID_ARGUMENT);

// Local CID invalid (0).
EXPECT_EQ(proxy
.AcquireBasicL2capChannel(/*connection_handle=*/0x123,
/*local_cid=*/0,
/*type=*/AclTransportType::kLe,
/*controller_receive_fn=*/nullptr)
.status(),
PW_STATUS_INVALID_ARGUMENT);
EXPECT_EQ(
proxy
.AcquireBasicL2capChannel(/*connection_handle=*/0x123,
/*local_cid=*/0,
/*type=*/AclTransportType::kLe,
/*payload_from_controller_fn=*/nullptr)
.status(),
PW_STATUS_INVALID_ARGUMENT);
}

TEST(BasicL2capChannelTest, BasicRead) {
Expand All @@ -2222,7 +2224,7 @@ TEST(BasicL2capChannelTest, BasicRead) {
/*connection_handle=*/handle,
/*local_cid=*/local_cid,
/*transport=*/AclTransportType::kLe,
/*controller_receive_fn=*/
/*payload_from_controller_fn=*/
[&capture](pw::span<uint8_t> payload) {
++capture.sends_called;
EXPECT_TRUE(std::equal(payload.begin(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class BasicL2capChannel : public L2capReadChannel {
L2capChannelManager& l2cap_channel_manager,
uint16_t connection_handle,
uint16_t local_cid,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn);
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn);

BasicL2capChannel(const BasicL2capChannel& other) = delete;
BasicL2capChannel& operator=(const BasicL2capChannel& other) = delete;
Expand All @@ -36,11 +37,11 @@ class BasicL2capChannel : public L2capReadChannel {
BasicL2capChannel& operator=(BasicL2capChannel&& other);

protected:
explicit BasicL2capChannel(
L2capChannelManager& l2cap_channel_manager,
uint16_t connection_handle,
uint16_t local_cid,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn);
explicit BasicL2capChannel(L2capChannelManager& l2cap_channel_manager,
uint16_t connection_handle,
uint16_t local_cid,
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn);

protected:
bool HandlePduFromController(pw::span<uint8_t> bframe) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class L2capReadChannel : public IntrusiveForwardList<L2capReadChannel>::Item {

// Handle an Rx L2CAP PDU.
//
// Implementations should call `CallControllerReceiveFn` after
// Implementations should call `SendPayloadFromControllerToClient` after
// recombining/processing the PDU (e.g. after updating channel state and
// screening out certain PDUs).
//
Expand All @@ -49,7 +49,7 @@ class L2capReadChannel : public IntrusiveForwardList<L2capReadChannel>::Item {

// Handle a Tx L2CAP PDU.
//
// Implementations should call `CallHostReceiveFn` after
// Implementations should call `SendPayloadFromHostToClient` after
// recombining/processing the PDU (e.g. after updating channel state and
// screening out certain PDUs).
//
Expand All @@ -69,21 +69,20 @@ class L2capReadChannel : public IntrusiveForwardList<L2capReadChannel>::Item {
uint16_t connection_handle() const { return connection_handle_; }

protected:
explicit L2capReadChannel(
L2capChannelManager& l2cap_channel_manager,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn,
uint16_t connection_handle,
uint16_t local_cid);
explicit L2capReadChannel(L2capChannelManager& l2cap_channel_manager,
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn,
uint16_t connection_handle,
uint16_t local_cid);

// Returns whether or not ACL connection handle & local L2CAP channel
// identifier are valid parameters for a packet.
[[nodiscard]] static bool AreValidParameters(uint16_t connection_handle,
uint16_t local_cid);

// Often the useful `payload` for clients is some subspan of the Rx SDU.
void CallControllerReceiveFn(pw::span<uint8_t> payload) {
if (controller_receive_fn_) {
controller_receive_fn_(payload);
void SendPayloadFromControllerToClient(pw::span<uint8_t> payload) {
if (payload_from_controller_fn_) {
payload_from_controller_fn_(payload);
}
}

Expand All @@ -95,7 +94,7 @@ class L2capReadChannel : public IntrusiveForwardList<L2capReadChannel>::Item {
// L2CAP channel ID of this channel.
uint16_t local_cid_;
// Client-provided controller read callback.
pw::Function<void(pw::span<uint8_t> payload)> controller_receive_fn_;
pw::Function<void(pw::span<uint8_t> payload)> payload_from_controller_fn_;

L2capChannelManager& l2cap_channel_manager_;
};
Expand Down
4 changes: 2 additions & 2 deletions pw_bluetooth_proxy/public/pw_bluetooth_proxy/l2cap_coc.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ class L2capCoc : public L2capWriteChannel, public L2capReadChannel {
pw::Function<void(pw::span<uint8_t> payload)>&& receive_fn,
pw::Function<void(Event event)>&& event_fn);

// `CallControllerReceiveFn` with the information payload contained in
// `kframe`. As packet desegmentation is not supported, segmented SDUs are
// `SendPayloadFromControllerToClient` with the information payload contained
// in `kframe`. As packet desegmentation is not supported, segmented SDUs are
// discarded.
bool HandlePduFromController(pw::span<uint8_t> kframe) override
PW_LOCKS_EXCLUDED(mutex_);
Expand Down
6 changes: 4 additions & 2 deletions pw_bluetooth_proxy/public/pw_bluetooth_proxy/proxy_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ class ProxyHost {
///
/// @param[in] transport Logical link transport type.
///
/// @param[in] controller_receive_fn Read callback to be invoked on Rx SDUs.
/// @param[in] payload_from_controller_fn Read callback to be invoked on Rx
/// SDUs.
///
/// @returns @rst
///
Expand All @@ -149,7 +150,8 @@ class ProxyHost {
uint16_t connection_handle,
uint16_t local_cid,
AclTransportType transport,
pw::Function<void(pw::span<uint8_t> payload)>&& controller_receive_fn);
pw::Function<void(pw::span<uint8_t> payload)>&&
payload_from_controller_fn);

/// Send a GATT Notify to the indicated connection.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class RfcommChannel final : public L2capWriteChannel, public L2capReadChannel {
Function<void(pw::span<uint8_t> payload)>&& receive_fn);

// Parses out RFCOMM payload from `l2cap_pdu` and calls
// `CallControllerReceiveFn`.
// `SendPayloadFromControllerToClient`.
bool HandlePduFromController(pw::span<uint8_t> l2cap_pdu) override;
bool HandlePduFromHost(pw::span<uint8_t> l2cap_pdu) override;

Expand Down
2 changes: 1 addition & 1 deletion pw_bluetooth_proxy/rfcomm_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ bool RfcommChannel::HandlePduFromController(pw::span<uint8_t> l2cap_pdu) {
const_cast<uint8_t*>(rfcomm_view->information().BackingStorage().data()),
rfcomm_view->information().SizeInBytes());

CallControllerReceiveFn(information);
SendPayloadFromControllerToClient(information);

bool rx_needs_refill = false;
{
Expand Down

0 comments on commit 027897c

Please sign in to comment.