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 <[email protected]> Reviewed-by: Erik Gilling <[email protected]> Pigweed-Auto-Submit: Anthony DiGirolamo <[email protected]> Reviewed-by: Keir Mierle <[email protected]>
google · Feb 10, 2024 · 4f06563 · 4f06563
1 parent 69cff2d
commit 4f06563
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 30 deletions.
diff --git a/pw_digital_io_rp2040/CMakeLists.txt 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
@@ -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
@@ -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 @@ using pw::digital_io::Rp2040DigitalInOut;
 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
@@ -20,15 +20,37 @@ Example code to use GPIO pins:
 
 .. 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;
-
-   Rp2040DigitalInOut out(/*gpio_pin=*/ 15);
+   using pw::digital_io::State;
+
+   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
@@ -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
@@ -67,6 +67,7 @@ generate_toolchain("rp2040") {
     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"