From ebb834102e8ba6a7afd4dc468037e6d8470e887e Mon Sep 17 00:00:00 2001 From: Zach Johnson Date: Wed, 11 Nov 2020 12:51:59 -0800 Subject: [PATCH] Simplify HAL protos * It's the HciHal, so HCI on functions and structs is redundant. * FetchXYZ returns a stream, so rename to StreamXYZ * Event & Commands are less about packets, so drop packet from the struct names Bug: 171749953 Tag: #gd-refactor Test: gd/cert/run --rhost Change-Id: I62fb098ee6a5a4805ad0667101c7de5a3368c4f2 --- gd/cert/py_hal.py | 8 ++++---- gd/hal/cert/simple_hal_test.py | 36 ++++++++++++++-------------------- gd/hal/facade.cc | 44 +++++++++++++++++++++--------------------- gd/hal/facade.proto | 28 +++++++++++++-------------- gd/rust/hal/src/facade.rs | 37 +++++++++++++---------------------- 5 files changed, 68 insertions(+), 85 deletions(-) diff --git a/gd/cert/py_hal.py b/gd/cert/py_hal.py index bce039e9f..8bdcad1eb 100644 --- a/gd/cert/py_hal.py +++ b/gd/cert/py_hal.py @@ -26,8 +26,8 @@ class PyHal(Closable): def __init__(self, device): self.device = device - self.hci_event_stream = EventStream(self.device.hal.FetchHciEvent(empty_proto.Empty())) - self.acl_stream = EventStream(self.device.hal.FetchHciAcl(empty_proto.Empty())) + self.hci_event_stream = EventStream(self.device.hal.StreamEvents(empty_proto.Empty())) + self.acl_stream = EventStream(self.device.hal.StreamAcl(empty_proto.Empty())) # We don't deal with SCO for now @@ -42,7 +42,7 @@ class PyHal(Closable): return self.acl_stream def send_hci_command(self, command): - self.device.hal.SendHciCommand(hal_facade.HciCommandPacket(payload=bytes(command))) + self.device.hal.SendCommand(hal_facade.Command(payload=bytes(command))) def send_acl(self, acl): - self.device.hal.SendHciAcl(hal_facade.HciAclPacket(payload=bytes(acl))) + self.device.hal.SendAcl(hal_facade.AclPacket(payload=bytes(acl))) diff --git a/gd/hal/cert/simple_hal_test.py b/gd/hal/cert/simple_hal_test.py index 385d39363..52e36b149 100644 --- a/gd/hal/cert/simple_hal_test.py +++ b/gd/hal/cert/simple_hal_test.py @@ -40,9 +40,7 @@ class SimpleHalTest(GdBaseTestClass): self.send_cert_hci_command(hci_packets.ResetBuilder()) def send_cert_hci_command(self, command): - self.cert.hal.SendHciCommand( - hal_facade_pb2.HciCommandPacket(payload=bytes(command.Serialize())), - timeout=_GRPC_TIMEOUT) + self.cert.hal.SendCommand(hal_facade_pb2.Command(payload=bytes(command.Serialize())), timeout=_GRPC_TIMEOUT) def send_cert_acl_data(self, handle, pb_flag, b_flag, acl): lower = handle & 0xff @@ -52,12 +50,10 @@ class SimpleHalTest(GdBaseTestClass): lower_length = len(acl) & 0xff upper_length = (len(acl) & 0xff00) >> 8 concatenated = bytes([lower, upper, lower_length, upper_length] + list(acl)) - self.cert.hal.SendHciAcl(hal_facade_pb2.HciAclPacket(payload=concatenated)) + self.cert.hal.SendAcl(hal_facade_pb2.AclPacket(payload=concatenated)) def send_dut_hci_command(self, command): - self.dut.hal.SendHciCommand( - hal_facade_pb2.HciCommandPacket(payload=bytes(command.Serialize())), - timeout=_GRPC_TIMEOUT) + self.dut.hal.SendCommand(hal_facade_pb2.Command(payload=bytes(command.Serialize())), timeout=_GRPC_TIMEOUT) def send_dut_acl_data(self, handle, pb_flag, b_flag, acl): lower = handle & 0xff @@ -67,16 +63,14 @@ class SimpleHalTest(GdBaseTestClass): lower_length = len(acl) & 0xff upper_length = (len(acl) & 0xff00) >> 8 concatenated = bytes([lower, upper, lower_length, upper_length] + list(acl)) - self.dut.hal.SendHciAcl( - hal_facade_pb2.HciAclPacket(payload=concatenated), - timeout=_GRPC_TIMEOUT) + self.dut.hal.SendAcl(hal_facade_pb2.AclPacket(payload=concatenated), timeout=_GRPC_TIMEOUT) def test_none_event(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream: assertThat(hci_event_stream).emitsNone(timeout=timedelta(seconds=1)) def test_fetch_hci_event(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream: self.send_dut_hci_command( hci_packets.LeAddDeviceToConnectListBuilder(hci_packets.ConnectListAddressType.RANDOM, @@ -86,7 +80,7 @@ class SimpleHalTest(GdBaseTestClass): assertThat(hci_event_stream).emits(lambda packet: bytes(event.Serialize()) in packet.payload) def test_loopback_hci_command(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream: self.send_dut_hci_command(hci_packets.WriteLoopbackModeBuilder(hci_packets.LoopbackMode.ENABLE_LOCAL)) @@ -97,7 +91,7 @@ class SimpleHalTest(GdBaseTestClass): assertThat(hci_event_stream).emits(lambda packet: bytes(command.Serialize()) in packet.payload) def test_inquiry_from_dut(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream: self.send_cert_hci_command(hci_packets.WriteScanEnableBuilder(hci_packets.ScanEnable.INQUIRY_AND_PAGE_SCAN)) lap = hci_packets.Lap() @@ -109,7 +103,7 @@ class SimpleHalTest(GdBaseTestClass): ) def test_le_ad_scan_cert_advertises(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream: # DUT scans self.send_dut_hci_command(hci_packets.LeSetRandomAddressBuilder('0D:05:04:03:02:01')) @@ -174,10 +168,10 @@ class SimpleHalTest(GdBaseTestClass): hci_packets.FilterDuplicates.DISABLED, 0, 0)) def test_le_connection_dut_advertises(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream, \ - EventStream(self.cert.hal.FetchHciEvent(empty_pb2.Empty())) as cert_hci_event_stream, \ - EventStream(self.dut.hal.FetchHciAcl(empty_pb2.Empty())) as acl_data_stream, \ - EventStream(self.cert.hal.FetchHciAcl(empty_pb2.Empty())) as cert_acl_data_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream, \ + EventStream(self.cert.hal.StreamEvents(empty_pb2.Empty())) as cert_hci_event_stream, \ + EventStream(self.dut.hal.StreamAcl(empty_pb2.Empty())) as acl_data_stream, \ + EventStream(self.cert.hal.StreamAcl(empty_pb2.Empty())) as cert_acl_data_stream: # Cert Connects self.send_cert_hci_command(hci_packets.LeSetRandomAddressBuilder('0C:05:04:03:02:01')) @@ -271,8 +265,8 @@ class SimpleHalTest(GdBaseTestClass): assertThat(acl_data_stream).emits(lambda packet: b'SomeMoreAclData' in packet.payload) def test_le_connect_list_connection_cert_advertises(self): - with EventStream(self.dut.hal.FetchHciEvent(empty_pb2.Empty())) as hci_event_stream, \ - EventStream(self.cert.hal.FetchHciEvent(empty_pb2.Empty())) as cert_hci_event_stream: + with EventStream(self.dut.hal.StreamEvents(empty_pb2.Empty())) as hci_event_stream, \ + EventStream(self.cert.hal.StreamEvents(empty_pb2.Empty())) as cert_hci_event_stream: # DUT Connects self.send_dut_hci_command(hci_packets.LeSetRandomAddressBuilder('0D:05:04:03:02:01')) diff --git a/gd/hal/facade.cc b/gd/hal/facade.cc index c5a7c736d..f1aba859c 100644 --- a/gd/hal/facade.cc +++ b/gd/hal/facade.cc @@ -40,9 +40,9 @@ class HciHalFacadeService : public HciHalFacade::Service, public ::bluetooth::ha hal_->unregisterIncomingPacketCallback(); } - ::grpc::Status SendHciCommand( + ::grpc::Status SendCommand( ::grpc::ServerContext* context, - const ::bluetooth::hal::HciCommandPacket* request, + const ::bluetooth::hal::Command* request, ::google::protobuf::Empty* response) override { std::unique_lock lock(mutex_); can_send_hci_command_ = false; @@ -54,55 +54,55 @@ class HciHalFacadeService : public HciHalFacade::Service, public ::bluetooth::ha return ::grpc::Status::OK; } - ::grpc::Status SendHciAcl( + ::grpc::Status SendAcl( ::grpc::ServerContext* context, - const ::bluetooth::hal::HciAclPacket* request, + const ::bluetooth::hal::AclPacket* request, ::google::protobuf::Empty* response) override { std::string req_string = request->payload(); hal_->sendAclData(std::vector(req_string.begin(), req_string.end())); return ::grpc::Status::OK; } - ::grpc::Status SendHciSco( + ::grpc::Status SendSco( ::grpc::ServerContext* context, - const ::bluetooth::hal::HciScoPacket* request, + const ::bluetooth::hal::ScoPacket* request, ::google::protobuf::Empty* response) override { std::string req_string = request->payload(); hal_->sendScoData(std::vector(req_string.begin(), req_string.end())); return ::grpc::Status::OK; } - ::grpc::Status FetchHciEvent( + ::grpc::Status StreamEvents( ::grpc::ServerContext* context, const ::google::protobuf::Empty* request, - ::grpc::ServerWriter* writer) override { + ::grpc::ServerWriter* writer) override { return pending_hci_events_.RunLoop(context, writer); }; - ::grpc::Status FetchHciAcl( + ::grpc::Status StreamAcl( ::grpc::ServerContext* context, const ::google::protobuf::Empty* request, - ::grpc::ServerWriter* writer) override { + ::grpc::ServerWriter* writer) override { return pending_acl_events_.RunLoop(context, writer); }; - ::grpc::Status FetchHciSco( + ::grpc::Status StreamSco( ::grpc::ServerContext* context, const ::google::protobuf::Empty* request, - ::grpc::ServerWriter* writer) override { + ::grpc::ServerWriter* writer) override { return pending_sco_events_.RunLoop(context, writer); }; - ::grpc::Status FetchHciIso( + ::grpc::Status StreamIso( ::grpc::ServerContext* context, const ::google::protobuf::Empty* request, - ::grpc::ServerWriter* writer) override { + ::grpc::ServerWriter* writer) override { return pending_iso_events_.RunLoop(context, writer); }; void hciEventReceived(bluetooth::hal::HciPacket event) override { { - HciEventPacket response; + Event response; response.set_payload(std::string(event.begin(), event.end())); pending_hci_events_.OnIncomingEvent(std::move(response)); } @@ -111,19 +111,19 @@ class HciHalFacadeService : public HciHalFacade::Service, public ::bluetooth::ha } void aclDataReceived(bluetooth::hal::HciPacket data) override { - HciAclPacket response; + AclPacket response; response.set_payload(std::string(data.begin(), data.end())); pending_acl_events_.OnIncomingEvent(std::move(response)); } void scoDataReceived(bluetooth::hal::HciPacket data) override { - HciScoPacket response; + ScoPacket response; response.set_payload(std::string(data.begin(), data.end())); pending_sco_events_.OnIncomingEvent(std::move(response)); } void isoDataReceived(bluetooth::hal::HciPacket data) override { - HciIsoPacket response; + IsoPacket response; response.set_payload(std::string(data.begin(), data.end())); pending_iso_events_.OnIncomingEvent(std::move(response)); } @@ -133,10 +133,10 @@ class HciHalFacadeService : public HciHalFacade::Service, public ::bluetooth::ha bool can_send_hci_command_ = true; mutable std::mutex mutex_; std::condition_variable cv_; - ::bluetooth::grpc::GrpcEventQueue pending_hci_events_{"FetchHciEvent"}; - ::bluetooth::grpc::GrpcEventQueue pending_acl_events_{"FetchHciAcl"}; - ::bluetooth::grpc::GrpcEventQueue pending_sco_events_{"FetchHciSco"}; - ::bluetooth::grpc::GrpcEventQueue pending_iso_events_{"FetchHciIso"}; + ::bluetooth::grpc::GrpcEventQueue pending_hci_events_{"StreamEvents"}; + ::bluetooth::grpc::GrpcEventQueue pending_acl_events_{"StreamAcl"}; + ::bluetooth::grpc::GrpcEventQueue pending_sco_events_{"StreamSco"}; + ::bluetooth::grpc::GrpcEventQueue pending_iso_events_{"StreamIso"}; }; void HciHalFacadeModule::ListDependencies(ModuleList* list) { diff --git a/gd/hal/facade.proto b/gd/hal/facade.proto index ddc43301f..8b2656d6d 100644 --- a/gd/hal/facade.proto +++ b/gd/hal/facade.proto @@ -5,33 +5,33 @@ package bluetooth.hal; import "google/protobuf/empty.proto"; service HciHalFacade { - rpc SendHciCommand(HciCommandPacket) returns (google.protobuf.Empty) {} - rpc SendHciAcl(HciAclPacket) returns (google.protobuf.Empty) {} - rpc SendHciSco(HciScoPacket) returns (google.protobuf.Empty) {} - rpc SendHciIso(HciIsoPacket) returns (google.protobuf.Empty) {} - - rpc FetchHciEvent(google.protobuf.Empty) returns (stream HciEventPacket) {} - rpc FetchHciAcl(google.protobuf.Empty) returns (stream HciAclPacket) {} - rpc FetchHciSco(google.protobuf.Empty) returns (stream HciScoPacket) {} - rpc FetchHciIso(google.protobuf.Empty) returns (stream HciIsoPacket) {} + rpc SendCommand(Command) returns (google.protobuf.Empty) {} + rpc SendAcl(AclPacket) returns (google.protobuf.Empty) {} + rpc SendSco(ScoPacket) returns (google.protobuf.Empty) {} + rpc SendIso(IsoPacket) returns (google.protobuf.Empty) {} + + rpc StreamEvents(google.protobuf.Empty) returns (stream Event) {} + rpc StreamAcl(google.protobuf.Empty) returns (stream AclPacket) {} + rpc StreamSco(google.protobuf.Empty) returns (stream ScoPacket) {} + rpc StreamIso(google.protobuf.Empty) returns (stream IsoPacket) {} } -message HciEventPacket { +message Event { bytes payload = 1; } -message HciCommandPacket { +message Command { bytes payload = 1; } -message HciAclPacket { +message AclPacket { bytes payload = 1; } -message HciScoPacket { +message ScoPacket { bytes payload = 1; } -message HciIsoPacket { +message IsoPacket { bytes payload = 1; } diff --git a/gd/rust/hal/src/facade.rs b/gd/rust/hal/src/facade.rs index 09f797bdf..19134d4e5 100644 --- a/gd/rust/hal/src/facade.rs +++ b/gd/rust/hal/src/facade.rs @@ -1,9 +1,8 @@ //! BT HCI HAL facade - -use bt_hal_proto::facade_grpc::{create_hci_hal_facade, HciHalFacade}; -use bt_hal_proto::facade::*; use bt_hal_proto::empty::Empty; +use bt_hal_proto::facade::*; +use bt_hal_proto::facade_grpc::{create_hci_hal_facade, HciHalFacade}; use tokio::runtime::Runtime; @@ -25,59 +24,49 @@ impl HciHalFacadeService { } impl HciHalFacade for HciHalFacadeService { - fn send_hci_command( - &mut self, - _ctx: RpcContext<'_>, - _cmd: HciCommandPacket, - _sink: UnarySink, - ) { + fn send_command(&mut self, _ctx: RpcContext<'_>, _cmd: Command, _sink: UnarySink) { unimplemented!() } - fn send_hci_acl(&mut self, _ctx: RpcContext<'_>, _acl: HciAclPacket, _sink: UnarySink) { + fn send_acl(&mut self, _ctx: RpcContext<'_>, _acl: AclPacket, _sink: UnarySink) { unimplemented!() } - fn send_hci_sco(&mut self, _ctx: RpcContext<'_>, _sco: HciScoPacket, _sink: UnarySink) { + fn send_sco(&mut self, _ctx: RpcContext<'_>, _sco: ScoPacket, _sink: UnarySink) { unimplemented!() } - fn send_hci_iso(&mut self, _ctx: RpcContext<'_>, _iso: HciIsoPacket, _sink: UnarySink) { + fn send_iso(&mut self, _ctx: RpcContext<'_>, _iso: IsoPacket, _sink: UnarySink) { unimplemented!() } - fn fetch_hci_event( - &mut self, - _ctx: RpcContext<'_>, - _: Empty, - _sink: ServerStreamingSink, - ) { + fn stream_events(&mut self, _ctx: RpcContext<'_>, _: Empty, _sink: ServerStreamingSink) { unimplemented!() } - fn fetch_hci_acl( + fn stream_acl( &mut self, _ctx: RpcContext<'_>, _: Empty, - _sink: ServerStreamingSink, + _sink: ServerStreamingSink, ) { unimplemented!() } - fn fetch_hci_sco( + fn stream_sco( &mut self, _ctx: RpcContext<'_>, _: Empty, - _sink: ServerStreamingSink, + _sink: ServerStreamingSink, ) { unimplemented!() } - fn fetch_hci_iso( + fn stream_iso( &mut self, _ctx: RpcContext<'_>, _: Empty, - _sink: ServerStreamingSink, + _sink: ServerStreamingSink, ) { unimplemented!() } -- 2.11.0