Skip to content

[RSDK-7151] Update analog reader responses to include accuracy data #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 20, 2024
Merged

[RSDK-7151] Update analog reader responses to include accuracy data #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
57116bd
start of change, doesn't compile yet
penguinland May 14, 2024
adbc018
make operator== for analog_value
penguinland May 14, 2024
7d78599
the names are min_range and max_range, not just min and max
penguinland May 14, 2024
b7b3d48
everything compiles
penguinland May 14, 2024
4a50390
fix lint errors
penguinland May 14, 2024
9c83730
this linter wants 2 spaces before an end-of-line comment
penguinland May 14, 2024
a751c87
don't modify status: it doesn't contain the extra data
penguinland May 17, 2024
4a1e52c
get everything to compile again
penguinland May 17, 2024
2d655c8
spaces, not tabs, in C++
penguinland May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/viam/sdk/components/board.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,23 @@ namespace sdk {
/// specific board implementations. This class cannot be used on its own.
class Board : public Component {
public:
/// @brief Represents the value received by the registered analog to digital converter (ADC).
/// The range and conversion mechanism to voltage will vary depending on the specific ADC
/// registered to the pin. Consult your ADC's documentation and Viam's `Board` documentation for
/// more details.
/// @brief Represents the raw value received by the registered analog to digital converter
/// (ADC). The range and conversion mechanism to voltage will vary depending on the specific
/// ADC registered to the pin.
using analog_value = int32_t;

/// @brief Represents the response received when reading the registered analog to digital
/// converter (ADC). The range and conversion mechanism to voltage will vary depending on the
/// specific ADC registered to the pin, though the min and max voltages and step_size can often
/// help with this conversion. Consult your ADC's documentation and Viam's `Board`
/// documentation for more details.
struct analog_response {
analog_value value;
float min_range; // Minimum possible voltage read by the analog reader
float max_range; // Maximum possible voltage read by the analog reader
float step_size; // Volts represented in each step in the value
};

/// @brief Depending on the type of digital interrupt, this can have different meanings. If a
/// `basic` (default) interrupt is configured, then this is the total interrupt count. If a
/// `servo` interrupt is configured this tracks the pulse width value. Consult Viam's `Board`
Expand Down Expand Up @@ -160,16 +171,16 @@ class Board : public Component {
/// @brief Reads off the current value of an analog reader on a board. Consult your ADC's docs
/// or Viam's `Board` docs for more information.
/// @param analog_reader_name analog reader to read from
inline analog_value read_analog(const std::string& analog_reader_name) {
inline analog_response read_analog(const std::string& analog_reader_name) {
return read_analog(analog_reader_name, {});
}

/// @brief Reads off the current value of an analog reader on a board. Consult your ADC's docs
/// or Viam's `Board` docs for more information.
/// @param analog_reader_name analog reader to read from
/// @param extra Any additional arguments to the method
virtual analog_value read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) = 0;
virtual analog_response read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) = 0;

/// @brief Writes the value to the analog writer of the board.
/// @param pin the pin to write to
Expand Down
7 changes: 4 additions & 3 deletions src/viam/sdk/components/private/board_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ AttributeMap BoardClient::do_command(const AttributeMap& command) {

// TODO(RSDK-6048) update `client_wrapper` to allow for requests without a `mutable_name()` method,
// then wrap here.
Board::analog_value BoardClient::read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) {
Board::analog_response BoardClient::read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) {
viam::component::board::v1::ReadAnalogReaderRequest request;
viam::component::board::v1::ReadAnalogReaderResponse response;
ClientContext ctx;
Expand All @@ -99,7 +99,8 @@ Board::analog_value BoardClient::read_analog(const std::string& analog_reader_na
if (!status.ok()) {
throw GRPCException(status);
}
return response.value();
return Board::analog_response{
response.value(), response.min_range(), response.max_range(), response.step_size()};
}

void BoardClient::write_analog(const std::string& pin, int value, const AttributeMap& extra) {
Expand Down
4 changes: 2 additions & 2 deletions src/viam/sdk/components/private/board_client.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class BoardClient : public Board {
void set_pwm_frequency(const std::string& pin,
uint64_t frequency_hz,
const AttributeMap& extra) override;
analog_value read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) override;
analog_response read_analog(const std::string& analog_reader_name,
const AttributeMap& extra) override;
void write_analog(const std::string& pin, int value, const AttributeMap& extra) override;
digital_value read_digital_interrupt(const std::string& digital_interrupt_name,
const AttributeMap& extra) override;
Expand Down
7 changes: 5 additions & 2 deletions src/viam/sdk/components/private/board_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,11 @@ ::grpc::Status BoardServer::ReadAnalogReader(
extra = struct_to_map(request->extra());
}

const Board::analog_value result = board->read_analog(request->analog_reader_name(), extra);
response->set_value(result);
const Board::analog_response result = board->read_analog(request->analog_reader_name(), extra);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I named the type analog_response, but named all the variables result. Should the type be analog_result instead? I'm unsure...

response->set_value(result.value);
response->set_min_range(result.min_range);
response->set_max_range(result.max_range);
response->set_step_size(result.step_size);

return ::grpc::Status();
}
Expand Down
4 changes: 2 additions & 2 deletions src/viam/sdk/tests/mocks/mock_board.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ AttributeMap MockBoard::do_command(const AttributeMap& command) {
return command;
}

Board::analog_value MockBoard::read_analog(const std::string& analog_reader_name,
const AttributeMap&) {
Board::analog_response MockBoard::read_analog(const std::string& analog_reader_name,
const AttributeMap&) {
this->peek_analog_reader_name = analog_reader_name;
return this->peek_read_analog_ret;
}
Expand Down
6 changes: 3 additions & 3 deletions src/viam/sdk/tests/mocks/mock_board.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class MockBoard : public viam::sdk::Board {
uint64_t frequency_hz,
const sdk::AttributeMap& extra) override;
viam::sdk::AttributeMap do_command(const viam::sdk::AttributeMap& command) override;
Board::analog_value read_analog(const std::string& analog_reader_name,
const sdk::AttributeMap& extra) override;
Board::analog_response read_analog(const std::string& analog_reader_name,
const sdk::AttributeMap& extra) override;
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;
Expand All @@ -44,7 +44,7 @@ class MockBoard : public viam::sdk::Board {
double peek_set_pwm_duty_cycle_pct;
uint64_t peek_get_pwm_frequency_ret;
uint64_t peek_set_pwm_frequency_hz;
Board::analog_value peek_read_analog_ret;
Board::analog_response peek_read_analog_ret;
Board::digital_value peek_read_digital_interrupt_ret;
Board::power_mode peek_set_power_mode_power_mode;
boost::optional<std::chrono::microseconds> peek_set_power_mode_duration;
Expand Down
9 changes: 7 additions & 2 deletions src/viam/sdk/tests/test_board.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,13 @@ BOOST_AUTO_TEST_CASE(test_do_command) {
BOOST_AUTO_TEST_CASE(test_read_analog) {
const auto mock = std::make_shared<MockBoard>("mock_board");
client_to_mock_pipeline<Board>(mock, [&](Board& client) {
mock->peek_read_analog_ret = 5150;
BOOST_CHECK_EQUAL(5150, client.read_analog("t1"));
sdk::Board::analog_response expected_result = {5150, 1.0, 5.0, 0.01};
mock->peek_read_analog_ret = expected_result;
sdk::Board::analog_response result = client.read_analog("t1");
BOOST_CHECK_EQUAL(expected_result.value, result.value);
BOOST_CHECK_EQUAL(expected_result.min_range, result.min_range);
BOOST_CHECK_EQUAL(expected_result.max_range, result.max_range);
BOOST_CHECK_EQUAL(expected_result.step_size, result.step_size);
BOOST_CHECK_EQUAL("t1", mock->peek_analog_reader_name);
});
}
Expand Down