From 0e426daae826a07f095f3933e797ce4b60d53f1a Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Fri, 5 Apr 2024 16:06:46 -0400 Subject: [PATCH 01/21] stream ticks --- src/viam/sdk/components/board.hpp | 25 ++++++++++- .../sdk/components/private/board_client.cpp | 27 +++++++++++ .../sdk/components/private/board_client.hpp | 5 +++ .../sdk/components/private/board_server.cpp | 45 +++++++++++++++++++ .../sdk/components/private/board_server.hpp | 4 ++ src/viam/sdk/tests/mocks/mock_board.cpp | 9 ++++ src/viam/sdk/tests/mocks/mock_board.hpp | 6 +++ src/viam/sdk/tests/test_board.cpp | 5 +++ 8 files changed, 125 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index baa0a293e..2ee2a280d 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -44,6 +45,15 @@ class Board : public Component { std::unordered_map digital_interrupt_values; }; + + /// @struct tick + /// A board digital interrupt that contains high or low and the time the digital interrupt occured. + struct tick { + std::string pin_name; + bool high; + uint64_t time; + }; + /// @enum power_mode /// @brief Power mode of the board /// The effect of these power modes depends on your physical board @@ -214,13 +224,26 @@ class Board : public Component { return read_digital_interrupt(digital_interrupt_name, {}); } - /// @brief Returns the current value of the interrupt which is based on the type of interrupt. + // @brief Returns the current value of the interrupt which is based on the type of interrupt. /// Consult Viam's `Board` docs for more information. /// @param digital_interrupt_name digital interrupt to check /// @param extra Any additional arguments to the method virtual digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const AttributeMap& extra) = 0; + /// @brief Returns a stream of digital interrupt ticks. + /// @param digital_interrupt_names digital interrupts to stream + inline void stream_ticks(const std::string digital_interrupt_names[], const std::queue ticks) { + return stream_ticks(digital_interrupt_names, ticks, {}); + } + + /// @brief Returns a stream of digital interrupt ticks. + /// @param digital_interrupt_names digital interrupts to stream + /// @param extra Any additional arguments to the method + virtual void stream_ticks(const std::string digital_interrupt_names[], + const std::queue ticks, + const AttributeMap& extra) = 0; + /// @brief Sets the power consumption mode of the board to the requested setting for the given /// duration. /// @param power_mode Requested power mode diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index 4644278a7..77b737acf 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -16,6 +16,8 @@ #include #include +using grpc::ClientReader; + namespace viam { namespace sdk { namespace impl { @@ -137,6 +139,31 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi return response.value(); } + +void BoardClient::stream_ticks(const std::string digital_interrupt_names[], + std::queue ticks, + const AttributeMap& extra) { + viam::component::board::v1::StreamTicksRequest request; + viam::component::board::v1::StreamTicksResponse response; + ClientContext ctx; + + request.set_name(this->name()); + request.set_pin_names(digital_interrupt_names); + *request.mutable_extra() = map_to_struct(extra); + + std::unique_ptr> reader( + stub_->StreamTicks(ctx, request, &response)); + while(reader->Read(&response)) { + Board::tick tick; + tick.pin_name = response.pin_name; + tick.high = response.high; + tick.time = response.time; + ticks.push(tick); + }; + + + } + void BoardClient::set_power_mode(power_mode power_mode, const AttributeMap& extra, const boost::optional& duration) { diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 5c0914c8e..1421f73df 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -45,6 +45,10 @@ class BoardClient : public Board { const boost::optional& duration) override; std::vector get_geometries(const AttributeMap& extra) override; + void stream_ticks(const std::string digital_interrupt_names[], + const std::queue ticks, + const AttributeMap& extra) override; + // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd // like to live in a world where implementers of derived classes don't need to go out of // their way to support two versions of a method (an `extra` version and a non-`extra` @@ -66,6 +70,7 @@ class BoardClient : public Board { using Board::set_pwm_duty_cycle; using Board::set_pwm_frequency; using Board::write_analog; + using Board::stream_ticks; private: using StubType = viam::component::board::v1::BoardService::StubInterface; diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 8f5a179f9..e809ca222 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -6,6 +6,9 @@ #include #include #include +#include + +using grpc::ServerWriter; namespace viam { namespace sdk { @@ -174,6 +177,48 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( return ::grpc::Status(); } + ::grpc::Status BoardServer::StreamTicks( + ::grpc::ServerContext*, + const ::viam::component::board::v1::StreamTicksRequest* request, + ::viam::component::board::v1::StreamTicksResponse* response) { + if (!request) { + return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, + "Called [Board::StreamTicks] without a request"); + }; + + const std::shared_ptr rb = resource_manager()->resource(request->name()); + if (!rb) { + return grpc::Status(grpc::UNKNOWN, "resource not found: " + request->name()); + } + + const std::shared_ptr board = std::dynamic_pointer_cast(rb); + + AttributeMap extra; + if (request->has_extra()) { + extra = struct_to_map(request->extra()); + } + + ServerWriter< ::viam::component::board::v1::StreamTicksResponse>* writer; + + std::queue ticks; + board->stream_ticks(request->digital_interrupt_names(), ticks, extra); + + + while(true) { + if(ticks.size() != 0) { + Board::tick tick = ticks.front(); + ticks.pop(); + response->pin_name = tick.pin_name; + response->high = tick.high; + response->time = tick.time; + writer->Write(response); + } + } + return ::grpc::Status(); + } + + + ::grpc::Status BoardServer::SetPowerMode( ::grpc::ServerContext*, const ::viam::component::board::v1::SetPowerModeRequest* request, diff --git a/src/viam/sdk/components/private/board_server.hpp b/src/viam/sdk/components/private/board_server.hpp index 49c41b757..95b4da8e8 100644 --- a/src/viam/sdk/components/private/board_server.hpp +++ b/src/viam/sdk/components/private/board_server.hpp @@ -74,6 +74,10 @@ class BoardServer : public ResourceServer, const ::viam::component::board::v1::GetDigitalInterruptValueRequest* request, ::viam::component::board::v1::GetDigitalInterruptValueResponse* response) override; + ::grpc::Status StreamTicks(::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, + ::viam::component::board::v1::StreamTicksResponse* response) override; + + ::grpc::Status SetPowerMode( ::grpc::ServerContext* context, const ::viam::component::board::v1::SetPowerModeRequest* request, diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 892b75cbc..e444edf43 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -71,6 +71,15 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita return this->peek_read_digital_interrupt_ret; } + + +void MockBoard::stream_ticks(const std::string digital_interrupt_names[], + std::queue ticks, + const AttributeMap& extra) { + for (int i = 0; i < sizeof(digital_interrupt_names); i++) { + this->peak_callbacks[digital_interrupt_names[i]] = ticks; + }} + void MockBoard::set_power_mode(power_mode power_mode, const AttributeMap&, const boost::optional& duration) { diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 1fc5c62b7..c6fae7aa3 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -2,6 +2,8 @@ #include #include +#include +#include namespace viam { namespace sdktests { @@ -27,6 +29,9 @@ class MockBoard : public viam::sdk::Board { void write_analog(const std::string& pin, int value, const sdk::AttributeMap& extra) override; Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; + void stream_ticks(const std::string digital_interrupt_names[], + std::queue ticks, + const AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, const boost::optional& duration) override; @@ -35,6 +40,7 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; Board::status peek_get_status_ret; + std::map peak_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index d16dc9035..dbd082d6b 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -146,6 +146,11 @@ BOOST_AUTO_TEST_CASE(test_read_digital_interrupt) { }); } +BOOST_AUTO_TEST_CASE(test_stream_ticks) { + const auto mock = std::make_shared("mock_board"); + +} + BOOST_AUTO_TEST_CASE(test_get_analog_reader_names) { const auto mock = std::make_shared("mock_board"); client_to_mock_pipeline(mock, [&](Board& client) { From daeaf5c4f216c252970e2cecb3defe646b5a76ed Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 9 Apr 2024 11:03:00 -0400 Subject: [PATCH 02/21] fix client add tests --- src/viam/sdk/components/board.hpp | 4 ++-- .../sdk/components/private/board_client.cpp | 19 ++++++++++--------- .../sdk/components/private/board_client.hpp | 2 +- .../sdk/components/private/board_server.cpp | 17 ++++++++++------- .../sdk/components/private/board_server.hpp | 2 +- src/viam/sdk/tests/mocks/mock_board.cpp | 11 +++++++---- src/viam/sdk/tests/mocks/mock_board.hpp | 6 +++--- src/viam/sdk/tests/test_board.cpp | 12 +++++++++++- 8 files changed, 45 insertions(+), 28 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 2ee2a280d..b8b0dcf0d 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -233,14 +233,14 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream - inline void stream_ticks(const std::string digital_interrupt_names[], const std::queue ticks) { + inline void stream_ticks(const std::vector digital_interrupt_names, const std::queue ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); } /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method - virtual void stream_ticks(const std::string digital_interrupt_names[], + virtual void stream_ticks(const std::vector digital_interrupt_names, const std::queue ticks, const AttributeMap& extra) = 0; diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index 77b737acf..bde41a642 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -140,7 +140,7 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi } -void BoardClient::stream_ticks(const std::string digital_interrupt_names[], +void BoardClient::stream_ticks(const std::vector digital_interrupt_names, std::queue ticks, const AttributeMap& extra) { viam::component::board::v1::StreamTicksRequest request; @@ -148,20 +148,21 @@ void BoardClient::stream_ticks(const std::string digital_interrupt_names[], ClientContext ctx; request.set_name(this->name()); - request.set_pin_names(digital_interrupt_names); + + for(int i = 0; i> reader = stub_->StreamTicks(ctx, request); - std::unique_ptr> reader( - stub_->StreamTicks(ctx, request, &response)); - while(reader->Read(&response)) { + while(reader->Read(&response)) { Board::tick tick; - tick.pin_name = response.pin_name; - tick.high = response.high; - tick.time = response.time; + tick.pin_name = response.pin_name(); + tick.high = response.high(); + tick.time = response.time(); ticks.push(tick); }; - } void BoardClient::set_power_mode(power_mode power_mode, diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 1421f73df..2083e9d02 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -45,7 +45,7 @@ class BoardClient : public Board { const boost::optional& duration) override; std::vector get_geometries(const AttributeMap& extra) override; - void stream_ticks(const std::string digital_interrupt_names[], + void stream_ticks(const std::vector digital_interrupt_names, const std::queue ticks, const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index e809ca222..5ebca4571 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -180,7 +180,7 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( ::grpc::Status BoardServer::StreamTicks( ::grpc::ServerContext*, const ::viam::component::board::v1::StreamTicksRequest* request, - ::viam::component::board::v1::StreamTicksResponse* response) { + ::grpc::ServerWriter< ::viam::component::board::v1::StreamTicksResponse>* writer) { if (!request) { return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, "Called [Board::StreamTicks] without a request"); @@ -198,19 +198,22 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( extra = struct_to_map(request->extra()); } - ServerWriter< ::viam::component::board::v1::StreamTicksResponse>* writer; - std::queue ticks; - board->stream_ticks(request->digital_interrupt_names(), ticks, extra); + google::protobuf::RepeatedPtrField pin_names = request->pin_names(); + const std::vector digital_interrupt_names(pin_names.begin(), pin_names.end()); + + board->stream_ticks(digital_interrupt_names, ticks, extra); + + ::viam::component::board::v1::StreamTicksResponse response; while(true) { if(ticks.size() != 0) { Board::tick tick = ticks.front(); ticks.pop(); - response->pin_name = tick.pin_name; - response->high = tick.high; - response->time = tick.time; + response.set_pin_name(tick.pin_name); + response.set_high(tick.high); + response.set_time(tick.time); writer->Write(response); } } diff --git a/src/viam/sdk/components/private/board_server.hpp b/src/viam/sdk/components/private/board_server.hpp index 95b4da8e8..3c12fc755 100644 --- a/src/viam/sdk/components/private/board_server.hpp +++ b/src/viam/sdk/components/private/board_server.hpp @@ -75,7 +75,7 @@ class BoardServer : public ResourceServer, ::viam::component::board::v1::GetDigitalInterruptValueResponse* response) override; ::grpc::Status StreamTicks(::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::viam::component::board::v1::StreamTicksResponse* response) override; + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) override; ::grpc::Status SetPowerMode( diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index e444edf43..422d39306 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -73,11 +73,14 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita -void MockBoard::stream_ticks(const std::string digital_interrupt_names[], - std::queue ticks, +void MockBoard::stream_ticks(const std::vector digital_interrupt_names, + std::queue ticks, const AttributeMap& extra) { - for (int i = 0; i < sizeof(digital_interrupt_names); i++) { - this->peak_callbacks[digital_interrupt_names[i]] = ticks; + for (int i = 0; i < digital_interrupt_names.size(); i++) { + std::cout << "heere di name" << digital_interrupt_names[i] << "\n"; + std::cout << "ticks address" << &ticks << "\n"; + this->peek_callbacks[digital_interrupt_names[i]] = ticks; + }} void MockBoard::set_power_mode(power_mode power_mode, diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index c6fae7aa3..190863b94 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -29,9 +29,9 @@ class MockBoard : public viam::sdk::Board { void write_analog(const std::string& pin, int value, const sdk::AttributeMap& extra) override; Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; - void stream_ticks(const std::string digital_interrupt_names[], + void stream_ticks(const std::vector digital_interrupt_names, std::queue ticks, - const AttributeMap& extra) override; + const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, const boost::optional& duration) override; @@ -40,7 +40,7 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; Board::status peek_get_status_ret; - std::map peak_callbacks; + std::map> peek_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index dbd082d6b..fa9bf8fe3 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -148,7 +148,17 @@ BOOST_AUTO_TEST_CASE(test_read_digital_interrupt) { BOOST_AUTO_TEST_CASE(test_stream_ticks) { const auto mock = std::make_shared("mock_board"); - + + client_to_mock_pipeline(mock, [&](Board& client) { + std::queue ticks; + std::vector pin_names = {"t1", "t2"}; + mock->sdk::Board::stream_ticks(pin_names, ticks); + + BOOST_CHECK_EQUAL(size(mock->peek_callbacks), 2); + BOOST_CHECK_EQUAL(mock->peek_callbacks.begin()->first, "t1"); + BOOST_CHECK_EQUAL(mock->peek_callbacks.end()->first, "t2"); + }); + } BOOST_AUTO_TEST_CASE(test_get_analog_reader_names) { From c402a9f6f31d3add99e9e0f3c62341f2ad2ede55 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 9 Apr 2024 17:40:23 -0400 Subject: [PATCH 03/21] use shared ptr --- src/viam/sdk/components/board.hpp | 4 ++-- .../sdk/components/private/board_client.cpp | 22 ++++++++++++------- .../sdk/components/private/board_client.hpp | 2 +- .../sdk/components/private/board_server.cpp | 13 ++++++----- src/viam/sdk/tests/mocks/mock_board.cpp | 6 ++--- src/viam/sdk/tests/mocks/mock_board.hpp | 2 +- src/viam/sdk/tests/test_board.cpp | 2 +- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index b8b0dcf0d..15c734121 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -233,7 +233,7 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream - inline void stream_ticks(const std::vector digital_interrupt_names, const std::queue ticks) { + inline void stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); } @@ -241,7 +241,7 @@ class Board : public Component { /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method virtual void stream_ticks(const std::vector digital_interrupt_names, - const std::queue ticks, + std::shared_ptr> ticks, const AttributeMap& extra) = 0; /// @brief Sets the power consumption mode of the board to the requested setting for the given diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index bde41a642..ff21b20ed 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -141,26 +141,32 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi void BoardClient::stream_ticks(const std::vector digital_interrupt_names, - std::queue ticks, + std::shared_ptr> ticks, const AttributeMap& extra) { + + std::cout << "here stream ticks" << std::endl; viam::component::board::v1::StreamTicksRequest request; viam::component::board::v1::StreamTicksResponse response; ClientContext ctx; + std::cout << "in client stream ticks" << std::endl; + request.set_name(this->name()); - for(int i = 0; i> reader = stub_->StreamTicks(ctx, request); + std::unique_ptr<::grpc::ClientReaderInterface<::viam::component::board::v1::StreamTicksResponse>> reader = stub_->StreamTicks(ctx, request); + while(reader->Read(&response)) { - Board::tick tick; - tick.pin_name = response.pin_name(); - tick.high = response.high(); - tick.time = response.time(); - ticks.push(tick); + Board::tick tick; + tick.pin_name = response.pin_name(); + tick.high = response.high(); + tick.time = response.time(); + ticks->push(tick); + reader->Finish(); }; } diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 2083e9d02..979aed277 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -46,7 +46,7 @@ class BoardClient : public Board { std::vector get_geometries(const AttributeMap& extra) override; void stream_ticks(const std::vector digital_interrupt_names, - const std::queue ticks, + const std::shared_ptr> ticks, const AttributeMap& extra) override; // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 5ebca4571..05934a775 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -178,7 +178,7 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( } ::grpc::Status BoardServer::StreamTicks( - ::grpc::ServerContext*, + ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, ::grpc::ServerWriter< ::viam::component::board::v1::StreamTicksResponse>* writer) { if (!request) { @@ -198,7 +198,7 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( extra = struct_to_map(request->extra()); } - std::queue ticks; + std::shared_ptr> ticks; google::protobuf::RepeatedPtrField pin_names = request->pin_names(); const std::vector digital_interrupt_names(pin_names.begin(), pin_names.end()); @@ -208,14 +208,17 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( ::viam::component::board::v1::StreamTicksResponse response; while(true) { - if(ticks.size() != 0) { - Board::tick tick = ticks.front(); - ticks.pop(); + if(ticks->size() != 0) { + Board::tick tick = ticks->front(); + ticks->pop(); response.set_pin_name(tick.pin_name); response.set_high(tick.high); response.set_time(tick.time); writer->Write(response); } + if (context->IsCancelled()) { + return grpc::Status(grpc::StatusCode::CANCELLED, "StreamTicks RPC is cancelled by the client"); + } } return ::grpc::Status(); } diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 422d39306..e3722819a 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -74,12 +74,10 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita void MockBoard::stream_ticks(const std::vector digital_interrupt_names, - std::queue ticks, + std::shared_ptr> ticks, const AttributeMap& extra) { for (int i = 0; i < digital_interrupt_names.size(); i++) { - std::cout << "heere di name" << digital_interrupt_names[i] << "\n"; - std::cout << "ticks address" << &ticks << "\n"; - this->peek_callbacks[digital_interrupt_names[i]] = ticks; + this->peek_callbacks[digital_interrupt_names[i]] = *ticks; }} diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 190863b94..3c04c49e6 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -30,7 +30,7 @@ class MockBoard : public viam::sdk::Board { Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; void stream_ticks(const std::vector digital_interrupt_names, - std::queue ticks, + std::shared_ptr> ticks, const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index fa9bf8fe3..823d9c066 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -150,7 +150,7 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { const auto mock = std::make_shared("mock_board"); client_to_mock_pipeline(mock, [&](Board& client) { - std::queue ticks; + std::shared_ptr> ticks; std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, ticks); From d79a732538c87a6ea2f2d4783d998bde85a936a2 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:12:37 -0400 Subject: [PATCH 04/21] lint --- src/viam/sdk/components/board.hpp | 15 +++++----- .../sdk/components/private/board_client.cpp | 22 +++++++------- .../sdk/components/private/board_client.hpp | 6 ++-- .../sdk/components/private/board_server.cpp | 29 +++++++++---------- .../sdk/components/private/board_server.hpp | 7 +++-- src/viam/sdk/tests/mocks/mock_board.cpp | 14 ++++----- src/viam/sdk/tests/mocks/mock_board.hpp | 8 ++--- src/viam/sdk/tests/test_board.cpp | 5 ++-- 8 files changed, 51 insertions(+), 55 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 15c734121..12d46dd1f 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -3,9 +3,9 @@ /// @brief Defines a `Board` component. #pragma once +#include #include #include -#include #include @@ -45,9 +45,9 @@ class Board : public Component { std::unordered_map digital_interrupt_values; }; - /// @struct tick - /// A board digital interrupt that contains high or low and the time the digital interrupt occured. + /// A board digital interrupt that contains high or low and the time the digital interrupt + /// occured. struct tick { std::string pin_name; bool high; @@ -224,7 +224,7 @@ class Board : public Component { return read_digital_interrupt(digital_interrupt_name, {}); } - // @brief Returns the current value of the interrupt which is based on the type of interrupt. + // @brief Returns the current value of the interrupt which is based on the type of interrupt. /// Consult Viam's `Board` docs for more information. /// @param digital_interrupt_name digital interrupt to check /// @param extra Any additional arguments to the method @@ -233,7 +233,8 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream - inline void stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks) { + inline void stream_ticks(const std::vector digital_interrupt_names, + std::shared_ptr> ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); } @@ -241,8 +242,8 @@ class Board : public Component { /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method virtual void stream_ticks(const std::vector digital_interrupt_names, - std::shared_ptr> ticks, - const AttributeMap& extra) = 0; + std::shared_ptr> ticks, + const AttributeMap& extra) = 0; /// @brief Sets the power consumption mode of the board to the requested setting for the given /// duration. diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index ff21b20ed..7fdb66236 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -139,11 +139,9 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi return response.value(); } - void BoardClient::stream_ticks(const std::vector digital_interrupt_names, - std::shared_ptr> ticks, - const AttributeMap& extra) { - + std::shared_ptr> ticks, + const AttributeMap& extra) { std::cout << "here stream ticks" << std::endl; viam::component::board::v1::StreamTicksRequest request; viam::component::board::v1::StreamTicksResponse response; @@ -153,23 +151,23 @@ void BoardClient::stream_ticks(const std::vector digital_interrupt_ request.set_name(this->name()); - for(int i = 0; i> reader = stub_->StreamTicks(ctx, request); - + std::unique_ptr< + ::grpc::ClientReaderInterface<::viam::component::board::v1::StreamTicksResponse>> + reader = stub_->StreamTicks(ctx, request); - while(reader->Read(&response)) { + while (reader->Read(&response)) { Board::tick tick; tick.pin_name = response.pin_name(); tick.high = response.high(); tick.time = response.time(); ticks->push(tick); reader->Finish(); - }; - - } + }; +} void BoardClient::set_power_mode(power_mode power_mode, const AttributeMap& extra, diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 979aed277..31c59eab6 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -46,8 +46,8 @@ class BoardClient : public Board { std::vector get_geometries(const AttributeMap& extra) override; void stream_ticks(const std::vector digital_interrupt_names, - const std::shared_ptr> ticks, - const AttributeMap& extra) override; + const std::shared_ptr> ticks, + const AttributeMap& extra) override; // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd // like to live in a world where implementers of derived classes don't need to go out of @@ -69,8 +69,8 @@ class BoardClient : public Board { using Board::set_power_mode; using Board::set_pwm_duty_cycle; using Board::set_pwm_frequency; - using Board::write_analog; using Board::stream_ticks; + using Board::write_analog; private: using StubType = viam::component::board::v1::BoardService::StubInterface; diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 05934a775..e61facb83 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -1,12 +1,12 @@ #include +#include #include #include #include #include #include #include -#include using grpc::ServerWriter; @@ -177,10 +177,10 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( return ::grpc::Status(); } - ::grpc::Status BoardServer::StreamTicks( +::grpc::Status BoardServer::StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::grpc::ServerWriter< ::viam::component::board::v1::StreamTicksResponse>* writer) { + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) { if (!request) { return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, "Called [Board::StreamTicks] without a request"); @@ -207,23 +207,22 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( ::viam::component::board::v1::StreamTicksResponse response; - while(true) { - if(ticks->size() != 0) { - Board::tick tick = ticks->front(); - ticks->pop(); - response.set_pin_name(tick.pin_name); - response.set_high(tick.high); - response.set_time(tick.time); - writer->Write(response); + while (true) { + if (ticks->size() != 0) { + Board::tick tick = ticks->front(); + ticks->pop(); + response.set_pin_name(tick.pin_name); + response.set_high(tick.high); + response.set_time(tick.time); + writer->Write(response); } if (context->IsCancelled()) { - return grpc::Status(grpc::StatusCode::CANCELLED, "StreamTicks RPC is cancelled by the client"); + return grpc::Status(grpc::StatusCode::CANCELLED, + "StreamTicks RPC is cancelled by the client"); } } return ::grpc::Status(); - } - - +} ::grpc::Status BoardServer::SetPowerMode( ::grpc::ServerContext*, diff --git a/src/viam/sdk/components/private/board_server.hpp b/src/viam/sdk/components/private/board_server.hpp index 3c12fc755..54fcbd1af 100644 --- a/src/viam/sdk/components/private/board_server.hpp +++ b/src/viam/sdk/components/private/board_server.hpp @@ -74,9 +74,10 @@ class BoardServer : public ResourceServer, const ::viam::component::board::v1::GetDigitalInterruptValueRequest* request, ::viam::component::board::v1::GetDigitalInterruptValueResponse* response) override; - ::grpc::Status StreamTicks(::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) override; - + ::grpc::Status StreamTicks( + ::grpc::ServerContext* context, + const ::viam::component::board::v1::StreamTicksRequest* request, + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) override; ::grpc::Status SetPowerMode( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index e3722819a..32d20025e 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -71,15 +71,13 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita return this->peek_read_digital_interrupt_ret; } - - void MockBoard::stream_ticks(const std::vector digital_interrupt_names, - std::shared_ptr> ticks, - const AttributeMap& extra) { - for (int i = 0; i < digital_interrupt_names.size(); i++) { - this->peek_callbacks[digital_interrupt_names[i]] = *ticks; - - }} + std::shared_ptr> ticks, + const AttributeMap& extra) { + for (int i = 0; i < digital_interrupt_names.size(); i++) { + this->peek_callbacks[digital_interrupt_names[i]] = *ticks; + } +} void MockBoard::set_power_mode(power_mode power_mode, const AttributeMap&, diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 3c04c49e6..b3a049743 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -1,9 +1,9 @@ #pragma once -#include -#include #include #include +#include +#include namespace viam { namespace sdktests { @@ -30,8 +30,8 @@ class MockBoard : public viam::sdk::Board { Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; void stream_ticks(const std::vector digital_interrupt_names, - std::shared_ptr> ticks, - const sdk::AttributeMap& extra) override; + std::shared_ptr> ticks, + const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, const boost::optional& duration) override; diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index 823d9c066..64466cdda 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(test_read_digital_interrupt) { BOOST_AUTO_TEST_CASE(test_stream_ticks) { const auto mock = std::make_shared("mock_board"); - client_to_mock_pipeline(mock, [&](Board& client) { + client_to_mock_pipeline(mock, [&](Board& client) { std::shared_ptr> ticks; std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, ticks); @@ -157,8 +157,7 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { BOOST_CHECK_EQUAL(size(mock->peek_callbacks), 2); BOOST_CHECK_EQUAL(mock->peek_callbacks.begin()->first, "t1"); BOOST_CHECK_EQUAL(mock->peek_callbacks.end()->first, "t2"); - }); - + }); } BOOST_AUTO_TEST_CASE(test_get_analog_reader_names) { From c838ddacbac4f8916c283e699efa7e27329fa831 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:20:27 -0400 Subject: [PATCH 05/21] cleanup --- src/viam/sdk/components/board.hpp | 6 ++++-- src/viam/sdk/components/private/board_client.cpp | 5 +---- src/viam/sdk/components/private/board_server.cpp | 6 ++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 12d46dd1f..3166f3f52 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -46,7 +46,7 @@ class Board : public Component { }; /// @struct tick - /// A board digital interrupt that contains high or low and the time the digital interrupt + /// A board digital interrupt that contains high/low value and the time the digital interrupt /// occured. struct tick { std::string pin_name; @@ -224,7 +224,7 @@ class Board : public Component { return read_digital_interrupt(digital_interrupt_name, {}); } - // @brief Returns the current value of the interrupt which is based on the type of interrupt. + /// @brief Returns the current value of the interrupt which is based on the type of interrupt. /// Consult Viam's `Board` docs for more information. /// @param digital_interrupt_name digital interrupt to check /// @param extra Any additional arguments to the method @@ -233,6 +233,7 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream + /// @param ticks queue to put the ticks in inline void stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); @@ -241,6 +242,7 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method + /// @param ticks queue to put the ticks in virtual void stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) = 0; diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index 7fdb66236..debef49e1 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -142,19 +142,17 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi void BoardClient::stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { - std::cout << "here stream ticks" << std::endl; viam::component::board::v1::StreamTicksRequest request; viam::component::board::v1::StreamTicksResponse response; ClientContext ctx; - std::cout << "in client stream ticks" << std::endl; - request.set_name(this->name()); for (int i = 0; i < digital_interrupt_names.size(); i++) { request.add_pin_names(digital_interrupt_names[i]); } *request.mutable_extra() = map_to_struct(extra); + std::unique_ptr< ::grpc::ClientReaderInterface<::viam::component::board::v1::StreamTicksResponse>> reader = stub_->StreamTicks(ctx, request); @@ -165,7 +163,6 @@ void BoardClient::stream_ticks(const std::vector digital_interrupt_ tick.high = response.high(); tick.time = response.time(); ticks->push(tick); - reader->Finish(); }; } diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index e61facb83..87190500a 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -200,15 +200,13 @@ ::grpc::Status BoardServer::StreamTicks( std::shared_ptr> ticks; - google::protobuf::RepeatedPtrField pin_names = request->pin_names(); - const std::vector digital_interrupt_names(pin_names.begin(), pin_names.end()); + const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); board->stream_ticks(digital_interrupt_names, ticks, extra); ::viam::component::board::v1::StreamTicksResponse response; - while (true) { - if (ticks->size() != 0) { + if (!ticks->empty()) { Board::tick tick = ticks->front(); ticks->pop(); response.set_pin_name(tick.pin_name); From 664276f851f992fd5ad71c3f2e732a71a7565389 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:07:18 -0400 Subject: [PATCH 06/21] format --- src/viam/sdk/components/private/board_server.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 87190500a..676c40d48 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -200,7 +200,8 @@ ::grpc::Status BoardServer::StreamTicks( std::shared_ptr> ticks; - const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); + const std::vector digital_interrupt_names(request->pin_names().begin(), + request->pin_names().end()); board->stream_ticks(digital_interrupt_names, ticks, extra); From 66218795f9b349100c5d1236b7c5c6ff9329fb3a Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:17:54 -0400 Subject: [PATCH 07/21] fix error --- src/viam/sdk/components/private/board_client.cpp | 2 +- src/viam/sdk/tests/mocks/mock_board.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index debef49e1..fe7f64fc7 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -148,7 +148,7 @@ void BoardClient::stream_ticks(const std::vector digital_interrupt_ request.set_name(this->name()); - for (int i = 0; i < digital_interrupt_names.size(); i++) { + for (unsigned int i = 0; i < digital_interrupt_names.size(); i++) { request.add_pin_names(digital_interrupt_names[i]); } *request.mutable_extra() = map_to_struct(extra); diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 32d20025e..dfeaa4eb2 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -74,7 +74,7 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita void MockBoard::stream_ticks(const std::vector digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { - for (int i = 0; i < digital_interrupt_names.size(); i++) { + for (unsigned int i = 0; i < digital_interrupt_names.size(); i++) { this->peek_callbacks[digital_interrupt_names[i]] = *ticks; } } From 5bc6fc55b3cb643f58b47441affb2b858414570e Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:24:55 -0400 Subject: [PATCH 08/21] fix cli error const --- src/viam/sdk/components/private/board_server.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 676c40d48..b671d4a34 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -198,8 +198,7 @@ ::grpc::Status BoardServer::StreamTicks( extra = struct_to_map(request->extra()); } - std::shared_ptr> ticks; - + const std::shared_ptr> ticks; const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); @@ -208,7 +207,7 @@ ::grpc::Status BoardServer::StreamTicks( ::viam::component::board::v1::StreamTicksResponse response; while (true) { if (!ticks->empty()) { - Board::tick tick = ticks->front(); + const Board::tick tick = ticks->front(); ticks->pop(); response.set_pin_name(tick.pin_name); response.set_high(tick.high); From 5b4f9473e09f0f89e8d6cb25303ecac4ef061650 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:37:20 -0400 Subject: [PATCH 09/21] size --- src/viam/sdk/tests/test_board.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index 64466cdda..b34fbebfe 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, ticks); - BOOST_CHECK_EQUAL(size(mock->peek_callbacks), 2); + BOOST_CHECK_EQUAL(std::size(mock->peek_callbacks), 2); BOOST_CHECK_EQUAL(mock->peek_callbacks.begin()->first, "t1"); BOOST_CHECK_EQUAL(mock->peek_callbacks.end()->first, "t2"); }); From 3c09baacb4c4f36091c916bdbe396f6cfcfb3119 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:53:11 -0400 Subject: [PATCH 10/21] fix --- src/viam/sdk/tests/test_board.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index b34fbebfe..6b7d1197c 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -154,7 +154,6 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, ticks); - BOOST_CHECK_EQUAL(std::size(mock->peek_callbacks), 2); BOOST_CHECK_EQUAL(mock->peek_callbacks.begin()->first, "t1"); BOOST_CHECK_EQUAL(mock->peek_callbacks.end()->first, "t2"); }); From b376c382ad623d55c2b5850d18fca0762082988c Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 10 Apr 2024 11:44:54 -0400 Subject: [PATCH 11/21] fix test --- src/viam/sdk/tests/mocks/mock_board.cpp | 2 +- src/viam/sdk/tests/mocks/mock_board.hpp | 2 +- src/viam/sdk/tests/test_board.cpp | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index dfeaa4eb2..a51133121 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -75,7 +75,7 @@ void MockBoard::stream_ticks(const std::vector digital_interrupt_na std::shared_ptr> ticks, const AttributeMap& extra) { for (unsigned int i = 0; i < digital_interrupt_names.size(); i++) { - this->peek_callbacks[digital_interrupt_names[i]] = *ticks; + this->peek_callbacks[digital_interrupt_names[i]] = ticks; } } diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index b3a049743..1cfe0a1c6 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -40,7 +40,7 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; Board::status peek_get_status_ret; - std::map> peek_callbacks; + std::map>> peek_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index 6b7d1197c..22192eac0 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -154,8 +154,10 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, ticks); - BOOST_CHECK_EQUAL(mock->peek_callbacks.begin()->first, "t1"); - BOOST_CHECK_EQUAL(mock->peek_callbacks.end()->first, "t2"); + auto iterator = mock->peek_callbacks.begin(); + BOOST_CHECK_EQUAL(iterator->first, "t1"); + iterator++; + BOOST_CHECK_EQUAL(iterator->first, "t2"); }); } From 8ea5142c2842e784adaf4bf6c4c74eb92f3198dd Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:11:55 -0400 Subject: [PATCH 12/21] pr comments --- src/viam/sdk/components/board.hpp | 9 ++--- .../sdk/components/private/board_client.cpp | 18 ++++------ .../sdk/components/private/board_client.hpp | 5 +-- .../sdk/components/private/board_server.cpp | 35 +++++++++---------- src/viam/sdk/tests/mocks/mock_board.cpp | 6 ++-- src/viam/sdk/tests/mocks/mock_board.hpp | 4 +-- 6 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 3166f3f52..4ae0e4007 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -50,8 +51,8 @@ class Board : public Component { /// occured. struct tick { std::string pin_name; + std::chrono::nanoseconds time; bool high; - uint64_t time; }; /// @enum power_mode @@ -234,8 +235,8 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream /// @param ticks queue to put the ticks in - inline void stream_ticks(const std::vector digital_interrupt_names, - std::shared_ptr> ticks) { + inline void stream_ticks(std::vector const& digital_interrupt_names, + std::shared_ptr> ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); } @@ -243,7 +244,7 @@ class Board : public Component { /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method /// @param ticks queue to put the ticks in - virtual void stream_ticks(const std::vector digital_interrupt_names, + virtual void stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) = 0; diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index fe7f64fc7..a64576ce4 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -16,7 +16,6 @@ #include #include -using grpc::ClientReader; namespace viam { namespace sdk { @@ -139,7 +138,8 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi return response.value(); } -void BoardClient::stream_ticks(const std::vector digital_interrupt_names, + +void BoardClient::stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { viam::component::board::v1::StreamTicksRequest request; @@ -148,21 +148,15 @@ void BoardClient::stream_ticks(const std::vector digital_interrupt_ request.set_name(this->name()); - for (unsigned int i = 0; i < digital_interrupt_names.size(); i++) { - request.add_pin_names(digital_interrupt_names[i]); + for (const auto& name: digital_interrupt_names) { + request.add_pin_names(name); } *request.mutable_extra() = map_to_struct(extra); - std::unique_ptr< - ::grpc::ClientReaderInterface<::viam::component::board::v1::StreamTicksResponse>> - reader = stub_->StreamTicks(ctx, request); + auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - Board::tick tick; - tick.pin_name = response.pin_name(); - tick.high = response.high(); - tick.time = response.time(); - ticks->push(tick); + ticks->push({std::move(response.pin_name()), std::chrono::nanoseconds(std::move(response.time())), std::move(response.high()) }); }; } diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 31c59eab6..9949ef7aa 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -45,8 +45,9 @@ class BoardClient : public Board { const boost::optional& duration) override; std::vector get_geometries(const AttributeMap& extra) override; - void stream_ticks(const std::vector digital_interrupt_names, - const std::shared_ptr> ticks, + + void stream_ticks(std::vector const& digital_interrupt_names, + std::shared_ptr> ticks, const AttributeMap& extra) override; // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index b671d4a34..d3b96d658 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -8,7 +8,6 @@ #include #include -using grpc::ServerWriter; namespace viam { namespace sdk { @@ -193,33 +192,31 @@ ::grpc::Status BoardServer::StreamTicks( const std::shared_ptr board = std::dynamic_pointer_cast(rb); - AttributeMap extra; - if (request->has_extra()) { - extra = struct_to_map(request->extra()); - } + ::viam::component::board::v1::StreamTicksResponse response; + std::shared_ptr> ticks; - const std::shared_ptr> ticks; - const std::vector digital_interrupt_names(request->pin_names().begin(), + make_service_helper( + "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { + const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); + board->stream_ticks(digital_interrupt_names, ticks, helper.getExtra()); + } + ); - board->stream_ticks(digital_interrupt_names, ticks, extra); - - ::viam::component::board::v1::StreamTicksResponse response; while (true) { - if (!ticks->empty()) { - const Board::tick tick = ticks->front(); - ticks->pop(); - response.set_pin_name(tick.pin_name); - response.set_high(tick.high); - response.set_time(tick.time); - writer->Write(response); - } if (context->IsCancelled()) { return grpc::Status(grpc::StatusCode::CANCELLED, "StreamTicks RPC is cancelled by the client"); } + if (!ticks->empty()) { + const auto& tick = ticks->front(); + ticks->pop(); + response.set_pin_name(std::move(tick.pin_name)); + response.set_high(std::move(tick.high)); + response.set_time(std::move(tick.time.count())); + writer->Write(response); + } } - return ::grpc::Status(); } ::grpc::Status BoardServer::SetPowerMode( diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index a51133121..88bc00687 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -71,11 +71,11 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita return this->peek_read_digital_interrupt_ret; } -void MockBoard::stream_ticks(const std::vector digital_interrupt_names, +void MockBoard::stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { - for (unsigned int i = 0; i < digital_interrupt_names.size(); i++) { - this->peek_callbacks[digital_interrupt_names[i]] = ticks; + for (const auto& name :digital_interrupt_names ) { + this->peek_callbacks[name] = *ticks; } } diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 1cfe0a1c6..eb71caa8f 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -29,7 +29,7 @@ class MockBoard : public viam::sdk::Board { void write_analog(const std::string& pin, int value, const sdk::AttributeMap& extra) override; Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; - void stream_ticks(const std::vector digital_interrupt_names, + void stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, @@ -40,7 +40,7 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; Board::status peek_get_status_ret; - std::map>> peek_callbacks; + std::map> peek_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; From ccec1442bb473c5dab205c556ba8e5fc561b8d89 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:15:22 -0400 Subject: [PATCH 13/21] clang tidy --- src/viam/sdk/components/board.hpp | 4 ++-- src/viam/sdk/components/private/board_client.cpp | 8 ++++---- src/viam/sdk/components/private/board_client.hpp | 1 - src/viam/sdk/components/private/board_server.cpp | 12 +++++------- src/viam/sdk/tests/mocks/mock_board.cpp | 2 +- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 4ae0e4007..7bcd5efd1 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -3,9 +3,9 @@ /// @brief Defines a `Board` component. #pragma once +#include #include #include -#include #include #include @@ -236,7 +236,7 @@ class Board : public Component { /// @param digital_interrupt_names digital interrupts to stream /// @param ticks queue to put the ticks in inline void stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks) { + std::shared_ptr> ticks) { return stream_ticks(digital_interrupt_names, ticks, {}); } diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index a64576ce4..cc559d8cb 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -16,7 +16,6 @@ #include #include - namespace viam { namespace sdk { namespace impl { @@ -138,7 +137,6 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi return response.value(); } - void BoardClient::stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { @@ -148,7 +146,7 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt request.set_name(this->name()); - for (const auto& name: digital_interrupt_names) { + for (const auto& name : digital_interrupt_names) { request.add_pin_names(name); } *request.mutable_extra() = map_to_struct(extra); @@ -156,7 +154,9 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - ticks->push({std::move(response.pin_name()), std::chrono::nanoseconds(std::move(response.time())), std::move(response.high()) }); + ticks->push({std::move(response.pin_name()), + std::chrono::nanoseconds(std::move(response.time())), + std::move(response.high())}); }; } diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index 9949ef7aa..d837c8a48 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -45,7 +45,6 @@ class BoardClient : public Board { const boost::optional& duration) override; std::vector get_geometries(const AttributeMap& extra) override; - void stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index d3b96d658..8a7985b1f 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -8,7 +8,6 @@ #include #include - namespace viam { namespace sdk { namespace impl { @@ -179,7 +178,7 @@ ::grpc::Status BoardServer::GetDigitalInterruptValue( ::grpc::Status BoardServer::StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) { + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept { if (!request) { return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, "Called [Board::StreamTicks] without a request"); @@ -197,11 +196,10 @@ ::grpc::Status BoardServer::StreamTicks( make_service_helper( "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { - const std::vector digital_interrupt_names(request->pin_names().begin(), - request->pin_names().end()); - board->stream_ticks(digital_interrupt_names, ticks, helper.getExtra()); - } - ); + const std::vector digital_interrupt_names(request->pin_names().begin(), + request->pin_names().end()); + board->stream_ticks(digital_interrupt_names, ticks, helper.getExtra()); + }); while (true) { if (context->IsCancelled()) { diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 88bc00687..9285ae5af 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -74,7 +74,7 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita void MockBoard::stream_ticks(std::vector const& digital_interrupt_names, std::shared_ptr> ticks, const AttributeMap& extra) { - for (const auto& name :digital_interrupt_names ) { + for (const auto& name : digital_interrupt_names) { this->peek_callbacks[name] = *ticks; } } From ef31d1a0bbd567087806008bac6c70eee8266fc3 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:25:00 -0400 Subject: [PATCH 14/21] fix lint error --- src/viam/sdk/components/private/board_client.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index cc559d8cb..5949cd853 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -154,9 +154,9 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - ticks->push({std::move(response.pin_name()), - std::chrono::nanoseconds(std::move(response.time())), - std::move(response.high())}); + ticks->push({response.pin_name(), + std::chrono::nanoseconds(response.time()), + response.high()}); }; } From d82746a5585b26e225fdc9d2f1a535e17e536b80 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:31:36 -0400 Subject: [PATCH 15/21] fix other lint issue --- src/viam/sdk/components/private/board_client.cpp | 5 ++--- src/viam/sdk/components/private/board_server.cpp | 6 +++--- src/viam/sdk/components/private/board_server.hpp | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index 5949cd853..d4983eebf 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -154,9 +154,8 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - ticks->push({response.pin_name(), - std::chrono::nanoseconds(response.time()), - response.high()}); + ticks->push( + {response.pin_name(), std::chrono::nanoseconds(response.time()), response.high()}); }; } diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 8a7985b1f..71d03e6cd 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -209,9 +209,9 @@ ::grpc::Status BoardServer::StreamTicks( if (!ticks->empty()) { const auto& tick = ticks->front(); ticks->pop(); - response.set_pin_name(std::move(tick.pin_name)); - response.set_high(std::move(tick.high)); - response.set_time(std::move(tick.time.count())); + response.set_pin_name(tick.pin_name); + response.set_high(tick.high); + response.set_time(tick.time.count()); writer->Write(response); } } diff --git a/src/viam/sdk/components/private/board_server.hpp b/src/viam/sdk/components/private/board_server.hpp index 54fcbd1af..a5e9dbb8f 100644 --- a/src/viam/sdk/components/private/board_server.hpp +++ b/src/viam/sdk/components/private/board_server.hpp @@ -77,7 +77,7 @@ class BoardServer : public ResourceServer, ::grpc::Status StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) override; + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept override; ::grpc::Status SetPowerMode( ::grpc::ServerContext* context, From 3aa8f2b86c59c1fe2e9e99cf57ba7329720b847d Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:46:19 -0400 Subject: [PATCH 16/21] fix failing test --- src/viam/sdk/components/private/board_server.hpp | 3 ++- src/viam/sdk/tests/mocks/mock_board.cpp | 2 +- src/viam/sdk/tests/mocks/mock_board.hpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/components/private/board_server.hpp b/src/viam/sdk/components/private/board_server.hpp index a5e9dbb8f..2e275d28e 100644 --- a/src/viam/sdk/components/private/board_server.hpp +++ b/src/viam/sdk/components/private/board_server.hpp @@ -77,7 +77,8 @@ class BoardServer : public ResourceServer, ::grpc::Status StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, - ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept override; + ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept + override; ::grpc::Status SetPowerMode( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 9285ae5af..52e115908 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -75,7 +75,7 @@ void MockBoard::stream_ticks(std::vector const& digital_interrupt_n std::shared_ptr> ticks, const AttributeMap& extra) { for (const auto& name : digital_interrupt_names) { - this->peek_callbacks[name] = *ticks; + this->peek_callbacks[name] = ticks; } } diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index eb71caa8f..653430e75 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -40,7 +40,7 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; Board::status peek_get_status_ret; - std::map> peek_callbacks; + std::map>> peek_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; From 93de94276731da65723e35e8d1eab7911053e03e Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Fri, 12 Apr 2024 17:52:10 -0400 Subject: [PATCH 17/21] use callbacks --- src/viam/sdk/components/board.hpp | 9 ++-- .../sdk/components/private/board_client.cpp | 10 +++-- .../sdk/components/private/board_client.hpp | 2 +- .../sdk/components/private/board_server.cpp | 42 ++++++------------- src/viam/sdk/tests/mocks/mock_board.cpp | 4 +- src/viam/sdk/tests/mocks/mock_board.hpp | 4 +- src/viam/sdk/tests/test_board.cpp | 7 +++- 7 files changed, 34 insertions(+), 44 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 7bcd5efd1..04623a083 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -49,7 +49,7 @@ class Board : public Component { /// @struct tick /// A board digital interrupt that contains high/low value and the time the digital interrupt /// occured. - struct tick { + struct Tick { std::string pin_name; std::chrono::nanoseconds time; bool high; @@ -236,16 +236,15 @@ class Board : public Component { /// @param digital_interrupt_names digital interrupts to stream /// @param ticks queue to put the ticks in inline void stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks) { - return stream_ticks(digital_interrupt_names, ticks, {}); + std::function& tick_handler) { + return stream_ticks(digital_interrupt_names, tick_handler, {}); } /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream /// @param extra Any additional arguments to the method - /// @param ticks queue to put the ticks in virtual void stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks, + std::function& tick_handler, const AttributeMap& extra) = 0; /// @brief Sets the power consumption mode of the board to the requested setting for the given diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index d4983eebf..d5b0640e6 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -138,7 +138,7 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi } void BoardClient::stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks, + std::function& tick_handler, const AttributeMap& extra) { viam::component::board::v1::StreamTicksRequest request; viam::component::board::v1::StreamTicksResponse response; @@ -154,8 +154,12 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - ticks->push( - {response.pin_name(), std::chrono::nanoseconds(response.time()), response.high()}); + Tick tick = + Tick{response.pin_name(), std::chrono::nanoseconds(response.time()), response.high()}; + bool stop = tick_handler(tick); + if (stop) { + return; + } }; } diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index d837c8a48..c3db839c6 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -46,7 +46,7 @@ class BoardClient : public Board { std::vector get_geometries(const AttributeMap& extra) override; void stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks, + std::function& tick_handler, const AttributeMap& extra) override; // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 71d03e6cd..72cecaa38 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -179,42 +179,26 @@ ::grpc::Status BoardServer::StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept { - if (!request) { - return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, - "Called [Board::StreamTicks] without a request"); - }; - - const std::shared_ptr rb = resource_manager()->resource(request->name()); - if (!rb) { - return grpc::Status(grpc::UNKNOWN, "resource not found: " + request->name()); - } - - const std::shared_ptr board = std::dynamic_pointer_cast(rb); - - ::viam::component::board::v1::StreamTicksResponse response; - std::shared_ptr> ticks; - - make_service_helper( + make_service_helper( "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); - board->stream_ticks(digital_interrupt_names, ticks, helper.getExtra()); - }); - - while (true) { - if (context->IsCancelled()) { - return grpc::Status(grpc::StatusCode::CANCELLED, - "StreamTicks RPC is cancelled by the client"); - } - if (!ticks->empty()) { - const auto& tick = ticks->front(); - ticks->pop(); + std::function writeTick = [writer, context](Board::Tick tick) { + if (context->IsCancelled()) { + // send bool to tell the board to stop calling the callback function. + return true; + } + ::viam::component::board::v1::StreamTicksResponse response; response.set_pin_name(tick.pin_name); response.set_high(tick.high); response.set_time(tick.time.count()); writer->Write(response); - } - } + return false; + }; + board->stream_ticks(digital_interrupt_names, writeTick, helper.getExtra()); + }); + + return ::grpc::Status(); } ::grpc::Status BoardServer::SetPowerMode( diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 52e115908..6e50d7624 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -72,10 +72,10 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita } void MockBoard::stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks, + std::function& tick_handler, const AttributeMap& extra) { for (const auto& name : digital_interrupt_names) { - this->peek_callbacks[name] = ticks; + peek_callbacks[name] = tick_handler; } } diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 653430e75..51bd8d176 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -30,7 +30,7 @@ class MockBoard : public viam::sdk::Board { Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; void stream_ticks(std::vector const& digital_interrupt_names, - std::shared_ptr> ticks, + std::function& tick_handler, const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, @@ -39,8 +39,8 @@ class MockBoard : public viam::sdk::Board { std::string peek_pin, peek_analog_reader_name, peek_digital_interrupt_name; int peek_pin_value; + std::map> peek_callbacks; Board::status peek_get_status_ret; - std::map>> peek_callbacks; bool peek_set_gpio_high; bool peek_get_gpio_ret; double peek_get_pwm_duty_cycle_ret; diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index 22192eac0..8b0a15c10 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -150,9 +150,12 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { const auto mock = std::make_shared("mock_board"); client_to_mock_pipeline(mock, [&](Board& client) { - std::shared_ptr> ticks; + std::function tick_handler = [](Board::Tick tick) -> bool { + return false; + }; + std::vector pin_names = {"t1", "t2"}; - mock->sdk::Board::stream_ticks(pin_names, ticks); + mock->sdk::Board::stream_ticks(pin_names, tick_handler); auto iterator = mock->peek_callbacks.begin(); BOOST_CHECK_EQUAL(iterator->first, "t1"); From 0fdffa8be0dffd6aaa40ebc08895dd87fb1f07cb Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Mon, 15 Apr 2024 10:37:00 -0400 Subject: [PATCH 18/21] remove unused import/fix docs comment --- src/viam/sdk/components/board.hpp | 4 ++-- src/viam/sdk/components/private/board_server.cpp | 3 +-- src/viam/sdk/tests/mocks/mock_board.hpp | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 04623a083..5bf965660 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -4,7 +4,6 @@ #pragma once #include -#include #include #include @@ -234,7 +233,7 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream - /// @param ticks queue to put the ticks in + /// @param tick_handler callback function to call when a tick occurs. inline void stream_ticks(std::vector const& digital_interrupt_names, std::function& tick_handler) { return stream_ticks(digital_interrupt_names, tick_handler, {}); @@ -242,6 +241,7 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream + /// @param tick_handler callback function to call when a tick occurs. /// @param extra Any additional arguments to the method virtual void stream_ticks(std::vector const& digital_interrupt_names, std::function& tick_handler, diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 72cecaa38..95b4897de 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -1,6 +1,5 @@ #include -#include #include #include #include @@ -179,7 +178,7 @@ ::grpc::Status BoardServer::StreamTicks( ::grpc::ServerContext* context, const ::viam::component::board::v1::StreamTicksRequest* request, ::grpc::ServerWriter<::viam::component::board::v1::StreamTicksResponse>* writer) noexcept { - make_service_helper( + make_service_helper( "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 51bd8d176..44c1974ee 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include From 8345d2546c4389654018610d43808ed24e2fd112 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 16 Apr 2024 10:34:16 -0400 Subject: [PATCH 19/21] pr comments --- src/viam/sdk/components/board.hpp | 12 ++++++++---- src/viam/sdk/components/private/board_client.cpp | 11 +++++------ src/viam/sdk/components/private/board_client.hpp | 2 +- src/viam/sdk/components/private/board_server.cpp | 14 +++++++------- src/viam/sdk/tests/mocks/mock_board.cpp | 2 +- src/viam/sdk/tests/mocks/mock_board.hpp | 2 +- src/viam/sdk/tests/test_board.cpp | 4 +--- 7 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 5bf965660..65dce67e6 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -46,11 +46,13 @@ class Board : public Component { }; /// @struct tick - /// A board digital interrupt that contains high/low value and the time the digital interrupt - /// occured. + /// @brief A board's digital interrupt. struct Tick { + // name of the digital interrupt pin. std::string pin_name; + // time in nanoseconds the tick occured. This does not represent an absolute time. std::chrono::nanoseconds time; + // bool high or low. bool high; }; @@ -234,8 +236,10 @@ class Board : public Component { /// @brief Returns a stream of digital interrupt ticks. /// @param digital_interrupt_names digital interrupts to stream /// @param tick_handler callback function to call when a tick occurs. + /// This should return true to keep streaming ticks and false to indicate that the stream of + /// ticks should terminate. The callback function should not be blocking. inline void stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler) { + std::function const& tick_handler) { return stream_ticks(digital_interrupt_names, tick_handler, {}); } @@ -244,7 +248,7 @@ class Board : public Component { /// @param tick_handler callback function to call when a tick occurs. /// @param extra Any additional arguments to the method virtual void stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler, + std::function const& tick_handler, const AttributeMap& extra) = 0; /// @brief Sets the power consumption mode of the board to the requested setting for the given diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index d5b0640e6..eec8bfe60 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -138,7 +138,7 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi } void BoardClient::stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler, + std::function const& tick_handler, const AttributeMap& extra) { viam::component::board::v1::StreamTicksRequest request; viam::component::board::v1::StreamTicksResponse response; @@ -154,11 +154,10 @@ void BoardClient::stream_ticks(std::vector const& digital_interrupt auto reader = stub_->StreamTicks(ctx, request); while (reader->Read(&response)) { - Tick tick = - Tick{response.pin_name(), std::chrono::nanoseconds(response.time()), response.high()}; - bool stop = tick_handler(tick); - if (stop) { - return; + if (!tick_handler({response.pin_name(), + std::chrono::nanoseconds(response.time()), + response.high()})) { + break; } }; } diff --git a/src/viam/sdk/components/private/board_client.hpp b/src/viam/sdk/components/private/board_client.hpp index c3db839c6..6a18c63fe 100644 --- a/src/viam/sdk/components/private/board_client.hpp +++ b/src/viam/sdk/components/private/board_client.hpp @@ -46,7 +46,7 @@ class BoardClient : public Board { std::vector get_geometries(const AttributeMap& extra) override; void stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler, + std::function const& tick_handler, const AttributeMap& extra) override; // the `extra` param is frequently unnecessary but needs to be supported. Ideally, we'd diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 95b4897de..b406d58e1 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -182,17 +182,17 @@ ::grpc::Status BoardServer::StreamTicks( "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); - std::function writeTick = [writer, context](Board::Tick tick) { + auto writeTick = [writer, context](Board::Tick tick) { if (context->IsCancelled()) { // send bool to tell the board to stop calling the callback function. - return true; + return false; } ::viam::component::board::v1::StreamTicksResponse response; - response.set_pin_name(tick.pin_name); - response.set_high(tick.high); - response.set_time(tick.time.count()); - writer->Write(response); - return false; + response.set_pin_name(std::move(tick.pin_name)); + response.set_high(std::move(tick.high)); + response.set_time(std::move(tick.time.count())); + writer->Write(std::move(response)); + return true; }; board->stream_ticks(digital_interrupt_names, writeTick, helper.getExtra()); }); diff --git a/src/viam/sdk/tests/mocks/mock_board.cpp b/src/viam/sdk/tests/mocks/mock_board.cpp index 6e50d7624..140557bcb 100644 --- a/src/viam/sdk/tests/mocks/mock_board.cpp +++ b/src/viam/sdk/tests/mocks/mock_board.cpp @@ -72,7 +72,7 @@ Board::digital_value MockBoard::read_digital_interrupt(const std::string& digita } void MockBoard::stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler, + std::function const& tick_handler, const AttributeMap& extra) { for (const auto& name : digital_interrupt_names) { peek_callbacks[name] = tick_handler; diff --git a/src/viam/sdk/tests/mocks/mock_board.hpp b/src/viam/sdk/tests/mocks/mock_board.hpp index 44c1974ee..7ff57852a 100644 --- a/src/viam/sdk/tests/mocks/mock_board.hpp +++ b/src/viam/sdk/tests/mocks/mock_board.hpp @@ -29,7 +29,7 @@ class MockBoard : public viam::sdk::Board { Board::digital_value read_digital_interrupt(const std::string& digital_interrupt_name, const sdk::AttributeMap& extra) override; void stream_ticks(std::vector const& digital_interrupt_names, - std::function& tick_handler, + std::function const& tick_handler, const sdk::AttributeMap& extra) override; void set_power_mode(power_mode power_mode, const sdk::AttributeMap& extra, diff --git a/src/viam/sdk/tests/test_board.cpp b/src/viam/sdk/tests/test_board.cpp index 8b0a15c10..c9efa8e22 100644 --- a/src/viam/sdk/tests/test_board.cpp +++ b/src/viam/sdk/tests/test_board.cpp @@ -150,9 +150,7 @@ BOOST_AUTO_TEST_CASE(test_stream_ticks) { const auto mock = std::make_shared("mock_board"); client_to_mock_pipeline(mock, [&](Board& client) { - std::function tick_handler = [](Board::Tick tick) -> bool { - return false; - }; + auto tick_handler = [](Board::Tick tick) -> bool { return false; }; std::vector pin_names = {"t1", "t2"}; mock->sdk::Board::stream_ticks(pin_names, tick_handler); From 323d1f18db9628a751daa7ab1b20980d035940b3 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 16 Apr 2024 13:44:51 -0400 Subject: [PATCH 20/21] pr --- src/viam/sdk/components/board.hpp | 8 +++++--- src/viam/sdk/components/private/board_server.cpp | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/viam/sdk/components/board.hpp b/src/viam/sdk/components/board.hpp index 65dce67e6..ee8ec9d1e 100644 --- a/src/viam/sdk/components/board.hpp +++ b/src/viam/sdk/components/board.hpp @@ -48,11 +48,13 @@ class Board : public Component { /// @struct tick /// @brief A board's digital interrupt. struct Tick { - // name of the digital interrupt pin. + /// name of the digital interrupt pin. std::string pin_name; - // time in nanoseconds the tick occured. This does not represent an absolute time. + + /// time in nanoseconds the tick occured. This does not represent an absolute time. std::chrono::nanoseconds time; - // bool high or low. + + /// bool high or low. bool high; }; diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index b406d58e1..81d7024a2 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -182,7 +182,7 @@ ::grpc::Status BoardServer::StreamTicks( "BoardServer::StreamTicks", this, request)([&](auto& helper, auto& board) { const std::vector digital_interrupt_names(request->pin_names().begin(), request->pin_names().end()); - auto writeTick = [writer, context](Board::Tick tick) { + auto writeTick = [writer, context](Board::Tick&& tick) { if (context->IsCancelled()) { // send bool to tell the board to stop calling the callback function. return false; From 923cf6f8ed2d3a88c399b048506e813d97bc07d4 Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 24 Apr 2024 16:10:37 -0400 Subject: [PATCH 21/21] lint error --- src/viam/sdk/components/private/board_server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index 81d7024a2..6dff9b735 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -191,7 +191,7 @@ ::grpc::Status BoardServer::StreamTicks( response.set_pin_name(std::move(tick.pin_name)); response.set_high(std::move(tick.high)); response.set_time(std::move(tick.time.count())); - writer->Write(std::move(response)); + writer->Write(response); return true; }; board->stream_ticks(digital_interrupt_names, writeTick, helper.getExtra());