pw_digital_io_rp2040: Config with polarity
- Allow specifying the pin and polarity in Rp2040Config.
- Return Status::FailedPrecondition() where appropriate.
- Add a few temp variables to improve readability.
- Address comments after this CL was merged:
https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/173550
Bug: 303255049
Change-Id: I72b6938bb86be285afee293531e8e6b3cf14202d
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/176290
Commit-Queue: Anthony DiGirolamo <tonymd@google.com>
Reviewed-by: Erik Gilling <konkers@google.com>
Pigweed-Auto-Submit: Anthony DiGirolamo <tonymd@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
diff --git a/pw_digital_io_rp2040/CMakeLists.txt b/pw_digital_io_rp2040/CMakeLists.txt
new file mode 100644
index 0000000..876c45b
--- /dev/null
+++ b/pw_digital_io_rp2040/CMakeLists.txt
@@ -0,0 +1,40 @@
+# Copyright 2023 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
+
+# This target provides the backend for pw::digital_io
+pw_add_library(pw_digital_io_rp2040 INTERFACE
+ HEADERS
+ public/pw_digital_io_rp2040/digital_io.h
+ PUBLIC_INCLUDES
+ public
+ SOURCES
+ digital_io.cc
+ PUBLIC_DEPS
+ pw_digital_io
+ pw_status
+ pw_third_party.rp2040
+)
+
+pw_add_test(pw_digital_io_rp2040.digital_io_test
+ SOURCES
+ digital_io_test.cc
+ PRIVATE_DEPS
+ pico_stdlib
+ pw_digital_io
+ GROUPS
+ modules
+ pw_digital_io_rp2040
+)
diff --git a/pw_digital_io_rp2040/digital_io.cc b/pw_digital_io_rp2040/digital_io.cc
index b6baeb7..695f4cf 100644
--- a/pw_digital_io_rp2040/digital_io.cc
+++ b/pw_digital_io_rp2040/digital_io.cc
@@ -15,50 +15,67 @@
#include "pw_digital_io_rp2040/digital_io.h"
#include "hardware/gpio.h"
-#include "pico/stdlib.h"
+#include "pw_digital_io/digital_io.h"
#include "pw_status/status.h"
namespace pw::digital_io {
-Rp2040DigitalIn::Rp2040DigitalIn(uint32_t pin) : pin_(pin) {}
+Rp2040DigitalIn::Rp2040DigitalIn(Rp2040Config config) : config_(config) {}
Status Rp2040DigitalIn::DoEnable(bool enable) {
if (!enable) {
- gpio_deinit(pin_);
+ gpio_deinit(config_.pin);
return OkStatus();
}
- gpio_init(pin_);
- gpio_set_dir(pin_, GPIO_IN);
+ gpio_init(config_.pin);
+ gpio_set_dir(config_.pin, GPIO_IN);
return OkStatus();
}
Result<State> Rp2040DigitalIn::DoGetState() {
- const pw::Result<State> result(State(gpio_get(pin_)));
- return result;
+ if (gpio_get_function(config_.pin) != GPIO_FUNC_SIO ||
+ gpio_get_dir(config_.pin) != GPIO_IN) {
+ return Status::FailedPrecondition();
+ }
+
+ const bool pin_value = gpio_get(config_.pin);
+ const State state = config_.PhysicalToLogical(pin_value);
+ return pw::Result<State>(state);
}
-Rp2040DigitalInOut::Rp2040DigitalInOut(uint32_t pin) : pin_(pin) {}
+Rp2040DigitalInOut::Rp2040DigitalInOut(Rp2040Config config) : config_(config) {}
Status Rp2040DigitalInOut::DoEnable(bool enable) {
if (!enable) {
- gpio_deinit(pin_);
+ gpio_deinit(config_.pin);
return OkStatus();
}
- gpio_init(pin_);
- gpio_set_dir(pin_, GPIO_OUT);
+ gpio_init(config_.pin);
+ gpio_set_dir(config_.pin, GPIO_OUT);
return OkStatus();
}
Status Rp2040DigitalInOut::DoSetState(State level) {
- gpio_put(pin_, level == State::kActive);
+ if (gpio_get_function(config_.pin) != GPIO_FUNC_SIO ||
+ gpio_get_dir(config_.pin) != GPIO_OUT) {
+ return Status::FailedPrecondition();
+ }
+
+ gpio_put(config_.pin, config_.LogicalToPhysical(level));
return OkStatus();
}
Result<State> Rp2040DigitalInOut::DoGetState() {
- const pw::Result<State> result(State(gpio_get(pin_)));
- return result;
+ if (gpio_get_function(config_.pin) != GPIO_FUNC_SIO ||
+ gpio_get_dir(config_.pin) != GPIO_OUT) {
+ return Status::FailedPrecondition();
+ }
+
+ const bool pin_value = gpio_get(config_.pin);
+ const State state = config_.PhysicalToLogical(pin_value);
+ return pw::Result<State>(state);
}
} // namespace pw::digital_io
diff --git a/pw_digital_io_rp2040/digital_io_test.cc b/pw_digital_io_rp2040/digital_io_test.cc
index 8ca9140..bdb4646 100644
--- a/pw_digital_io_rp2040/digital_io_test.cc
+++ b/pw_digital_io_rp2040/digital_io_test.cc
@@ -14,6 +14,7 @@
#include "pw_digital_io/digital_io.h"
#include "pw_digital_io_rp2040/digital_io.h"
+#include "pw_result/result.h"
#include "pw_unit_test/framework.h"
using pw::digital_io::Rp2040DigitalIn;
@@ -22,17 +23,38 @@
namespace pw::digital_io {
namespace {
-Rp2040DigitalInOut output_pin(/*gpio_pin=*/15);
-Rp2040DigitalIn input_pin(/*gpio_pin=*/16);
+constexpr Rp2040Config output_pin_config{
+ .pin = 15,
+ .polarity = Polarity::kActiveLow,
+};
+constexpr Rp2040Config input_pin_config{
+ .pin = 16,
+ .polarity = Polarity::kActiveHigh,
+};
+Rp2040DigitalInOut output_pin(output_pin_config);
+Rp2040DigitalIn input_pin(input_pin_config);
+
+TEST(DigitalIoTest, PhysicalToLogical) {
+ ASSERT_EQ(State::kActive, output_pin_config.PhysicalToLogical(false));
+ ASSERT_EQ(State::kInactive, output_pin_config.PhysicalToLogical(true));
+ ASSERT_EQ(State::kActive, input_pin_config.PhysicalToLogical(true));
+ ASSERT_EQ(State::kInactive, input_pin_config.PhysicalToLogical(false));
+}
+
+TEST(DigitalIoTest, LogicalToPhysical) {
+ ASSERT_EQ(false, output_pin_config.LogicalToPhysical(State::kActive));
+ ASSERT_EQ(true, output_pin_config.LogicalToPhysical(State::kInactive));
+ ASSERT_EQ(true, input_pin_config.LogicalToPhysical(State::kActive));
+ ASSERT_EQ(false, input_pin_config.LogicalToPhysical(State::kInactive));
+}
TEST(DigitalIoTest, Init) {
// Simple test only meant to ensure module is compiled.
output_pin.Enable();
- output_pin.SetState(pw::digital_io::State::kInactive);
output_pin.SetState(pw::digital_io::State::kActive);
input_pin.Enable();
- auto state_result = input_pin.GetState();
+ Result<State> state_result = input_pin.GetState();
ASSERT_EQ(OkStatus(), state_result.status());
ASSERT_EQ(State::kInactive, state_result.value());
}
diff --git a/pw_digital_io_rp2040/docs.rst b/pw_digital_io_rp2040/docs.rst
index 6bc017e..6ec52c2 100644
--- a/pw_digital_io_rp2040/docs.rst
+++ b/pw_digital_io_rp2040/docs.rst
@@ -20,15 +20,37 @@
.. code-block:: cpp
+ #include "pw_digital_io/polarity.h"
#include "pw_digital_io_rp2040/digital_io.h"
+ using pw::digital_io::Polarity;
+ using pw::digital_io::Rp2040Config;
using pw::digital_io::Rp2040DigitalIn;
using pw::digital_io::Rp2040DigitalInOut;
+ using pw::digital_io::State;
- Rp2040DigitalInOut out(/*gpio_pin=*/ 15);
+ constexpr Rp2040Config output_pin_config{
+ .pin = 15,
+ .polarity = Polarity::kActiveLow,
+ };
+ constexpr Rp2040Config input_pin_config{
+ .pin = 16,
+ .polarity = Polarity::kActiveHigh,
+ };
+
+ // Config output pin:
+ Rp2040DigitalInOut out(output_pin_config);
out.Enable();
- out.SetState(pw::digital_io::State::kInactive);
- Rp2040DigitalIn in(/*gpio_pin=*/ 16);
+ // Set the output pin active.
+ // This pulls pin to ground since the polarity is kActiveLow.
+ out.SetState(State::kActive);
+
+ // Config input pin:
+ Rp2040DigitalIn in(input_pin_config);
in.Enable();
- auto state = in.GetState();
+
+ // Get the pin state. Since the polarity is kActiveHigh this will return
+ // State::kActive if the pin is high or and State::kInactive if the pin is
+ // low (grounded).
+ State pin_state = in.GetState();
diff --git a/pw_digital_io_rp2040/public/pw_digital_io_rp2040/digital_io.h b/pw_digital_io_rp2040/public/pw_digital_io_rp2040/digital_io.h
index 4d96f35..3d4c09e 100644
--- a/pw_digital_io_rp2040/public/pw_digital_io_rp2040/digital_io.h
+++ b/pw_digital_io_rp2040/public/pw_digital_io_rp2040/digital_io.h
@@ -1,4 +1,4 @@
-// Copyright 2022 The Pigweed Authors
+// Copyright 2023 The Pigweed Authors
//
// Licensed under the Apache License, Version 2.0 (the "License"); you may not
// use this file except in compliance with the License. You may obtain a copy of
@@ -17,30 +17,47 @@
#include <cstdint>
#include "pw_digital_io/digital_io.h"
+#include "pw_digital_io/polarity.h"
namespace pw::digital_io {
+struct Rp2040Config {
+ uint16_t pin;
+ Polarity polarity;
+
+ bool operator==(const Rp2040Config& rhs) const {
+ return polarity == rhs.polarity && pin == rhs.pin;
+ }
+ State PhysicalToLogical(const bool hal_value) const {
+ return polarity == Polarity::kActiveHigh ? State(hal_value)
+ : State(!hal_value);
+ }
+ bool LogicalToPhysical(const State state) const {
+ return polarity == Polarity::kActiveHigh ? (bool)state : !(bool)state;
+ }
+};
+
class Rp2040DigitalInOut : public DigitalInOut {
public:
- Rp2040DigitalInOut(uint32_t pin);
+ Rp2040DigitalInOut(Rp2040Config config);
+ private:
Status DoEnable(bool enable) override;
Status DoSetState(State level) override;
Result<State> DoGetState() override;
- private:
- uint32_t pin_;
+ Rp2040Config config_;
};
class Rp2040DigitalIn : public DigitalIn {
public:
- Rp2040DigitalIn(uint32_t pin);
+ Rp2040DigitalIn(Rp2040Config config);
+ private:
Status DoEnable(bool enable) override;
Result<State> DoGetState() override;
- private:
- uint32_t pin_;
+ Rp2040Config config_;
};
} // namespace pw::digital_io
diff --git a/targets/rp2040/BUILD.gn b/targets/rp2040/BUILD.gn
index 9e2c9e5..7e59ad1 100644
--- a/targets/rp2040/BUILD.gn
+++ b/targets/rp2040/BUILD.gn
@@ -67,6 +67,7 @@
pw_assert_BACKEND = dir_pw_assert_basic
pw_log_BACKEND = dir_pw_log_basic
pw_sys_io_BACKEND = dir_pw_sys_io_rp2040
+ pw_rpc_system_server_BACKEND = "$dir_pw_hdlc:hdlc_sys_io_system_server"
pw_sync_INTERRUPT_SPIN_LOCK_BACKEND =
"$dir_pw_sync_baremetal:interrupt_spin_lock"
