Don't discard Status returns
Bug: 357682801
Change-Id: I90775cce0d448c799e0ec33fdf1df191c97ded24
Reviewed-on: https://pigweed-internal-review.git.corp.google.com/c/pigweed/showcase/rp2/+/73957
Commit-Queue: Ted Pudlik <tpudlik@google.com>
Reviewed-by: Aaron Green <aarongreen@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-internal-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/apps/factory/service.cc b/apps/factory/service.cc
index 71e968d..af8b28d 100644
--- a/apps/factory/service.cc
+++ b/apps/factory/service.cc
@@ -50,16 +50,13 @@
break;
case factory_Test_Type_LTR559_PROX:
PW_LOG_INFO("Configured for LTR559 proximity test");
- proximity_sensor_->Enable();
- break;
+ return proximity_sensor_->Enable();
case factory_Test_Type_LTR559_LIGHT:
PW_LOG_INFO("Configured for LTR559 ambient light test");
- ambient_light_sensor_->Enable();
- break;
+ return ambient_light_sensor_->Enable();
case factory_Test_Type_BME688:
PW_LOG_INFO("Configured for BME688 air sensor test");
- air_sensor_->Init();
- break;
+ return air_sensor_->Init();
}
return pw::OkStatus();
@@ -72,11 +69,9 @@
button_manager_->Stop();
break;
case factory_Test_Type_LTR559_PROX:
- proximity_sensor_->Disable();
- break;
+ return proximity_sensor_->Disable();
case factory_Test_Type_LTR559_LIGHT:
- ambient_light_sensor_->Disable();
- break;
+ return ambient_light_sensor_->Disable();
case factory_Test_Type_BME688:
// Do nothing.
break;
diff --git a/apps/production/BUILD.bazel b/apps/production/BUILD.bazel
index 3b246fb..9304a23 100644
--- a/apps/production/BUILD.bazel
+++ b/apps/production/BUILD.bazel
@@ -16,7 +16,7 @@
load("@pigweed//targets/host_device_simulator:transition.bzl", "host_device_simulator_binary")
load("@pigweed//targets/rp2040:flash.bzl", "flash_rp2040")
load("//targets/rp2:binary.bzl", "rp2040_binary", "rp2350_binary")
-load("//tools:tools.bzl", "sense_device_console", "sense_host_console",)
+load("//tools:tools.bzl", "sense_device_console", "sense_host_console")
package(default_visibility = ["//visibility:public"])
@@ -36,6 +36,7 @@
"//system:worker",
"//system",
":threads",
+ "@pigweed//pw_assert:check",
"@pigweed//pw_log",
"@pigweed//pw_system:async",
"@pigweed//pw_thread:thread",
@@ -102,7 +103,7 @@
sense_device_console(
name = "rp2040_webconsole",
binary = ":rp2040.elf",
- extra_args = ["--browser"]
+ extra_args = ["--browser"],
)
sense_device_console(
@@ -113,7 +114,7 @@
sense_device_console(
name = "rp2350_webconsole",
binary = ":rp2350.elf",
- extra_args = ["--browser"]
+ extra_args = ["--browser"],
)
alias(
diff --git a/apps/production/main.cc b/apps/production/main.cc
index c181b54..71f0840 100644
--- a/apps/production/main.cc
+++ b/apps/production/main.cc
@@ -44,9 +44,9 @@
void InitEventTimers() {
auto& pubsub = system::PubSub();
static EventTimers<3> event_timers(pubsub);
- event_timers.AddEventTimer(StateManager::kRepeatAlarmToken);
- event_timers.AddEventTimer(StateManager::kSilenceAlarmToken);
- event_timers.AddEventTimer(StateManager::kThresholdModeToken);
+ PW_CHECK_OK(event_timers.AddEventTimer(StateManager::kRepeatAlarmToken));
+ PW_CHECK_OK(event_timers.AddEventTimer(StateManager::kSilenceAlarmToken));
+ PW_CHECK_OK(event_timers.AddEventTimer(StateManager::kThresholdModeToken));
}
void InitBoardService() {
@@ -68,8 +68,8 @@
PW_CHECK(system::PubSub().SubscribeTo<MorseEncodeRequest>(
[](MorseEncodeRequest request) {
- morse_encoder.Encode(
- request.message, request.repeat, Encoder::kDefaultIntervalMs);
+ PW_CHECK_OK(morse_encoder.Encode(
+ request.message, request.repeat, Encoder::kDefaultIntervalMs));
}));
}
diff --git a/modules/air_sensor/BUILD.bazel b/modules/air_sensor/BUILD.bazel
index 84da72d..9ef9931 100644
--- a/modules/air_sensor/BUILD.bazel
+++ b/modules/air_sensor/BUILD.bazel
@@ -99,6 +99,7 @@
":air_sensor",
":nanopb_rpc",
"//modules/worker",
+ "@pigweed//pw_assert:check",
"@pigweed//pw_chrono:system_clock",
"@pigweed//pw_chrono:system_timer",
"@pigweed//pw_sync:thread_notification",
diff --git a/modules/air_sensor/air_sensor_test.cc b/modules/air_sensor/air_sensor_test.cc
index f9b28f5..ab23d9a 100644
--- a/modules/air_sensor/air_sensor_test.cc
+++ b/modules/air_sensor/air_sensor_test.cc
@@ -28,7 +28,7 @@
protected:
using Score = AirSensor::Score;
- void SetUp() override { air_sensor_.Init(); }
+ void SetUp() override { ASSERT_EQ(pw::OkStatus(), air_sensor_.Init()); }
void MeasureRepeated(size_t n) {
for (size_t i = 0; i < n; ++i) {
@@ -130,7 +130,7 @@
});
for (size_t i = 0; i < 3; ++i) {
- air_sensor_.Measure(response_);
+ ASSERT_EQ(pw::OkStatus(), air_sensor_.Measure(response_));
request_.release();
response_.acquire();
EXPECT_EQ(air_sensor_.temperature(), AirSensor::kDefaultTemperature + i);
diff --git a/modules/air_sensor/service.cc b/modules/air_sensor/service.cc
index e6f565f..f12e931 100644
--- a/modules/air_sensor/service.cc
+++ b/modules/air_sensor/service.cc
@@ -14,6 +14,7 @@
#include "modules/air_sensor/service.h"
+#include "pw_assert/check.h"
#include "pw_log/log.h"
namespace sense {
@@ -38,7 +39,10 @@
const air_sensor_MeasureStreamRequest& request,
ServerWriter<air_sensor_Measurement>& writer) {
if (request.sample_interval_ms < 500) {
- writer.Finish(pw::Status::InvalidArgument());
+ if (const auto status = writer.Finish(pw::Status::InvalidArgument());
+ !status.ok()) {
+ PW_LOG_ERROR("Failed to write response: %s", status.str());
+ }
return;
}
diff --git a/modules/blinky/BUILD.bazel b/modules/blinky/BUILD.bazel
index 760370a..6c21c29 100644
--- a/modules/blinky/BUILD.bazel
+++ b/modules/blinky/BUILD.bazel
@@ -67,6 +67,7 @@
deps = [
":blinky",
":nanopb_rpc",
+ "@pigweed//pw_assert:check",
"@pigweed//pw_allocator:allocator",
"@pigweed//pw_async2:dispatcher",
],
diff --git a/modules/blinky/service.cc b/modules/blinky/service.cc
index 44b1e7e..6ba158e 100644
--- a/modules/blinky/service.cc
+++ b/modules/blinky/service.cc
@@ -13,6 +13,7 @@
// the License.
#define PW_LOG_MODULE_NAME "BLINKY"
+#include "pw_assert/check.h"
#include "modules/blinky/service.h"
namespace sense {
@@ -23,7 +24,7 @@
PolychromeLed& polychrome_led) {
blinky_.Init(dispatcher, allocator, monochrome_led, polychrome_led);
// Start binking once every 1000ms.
- blinky_.Blink(/*blink_count=*/0, /*interval_ms=*/1000);
+ PW_CHECK_OK(blinky_.Blink(/*blink_count=*/0, /*interval_ms=*/1000));
}
pw::Status BlinkyService::ToggleLed(const pw_protobuf_Empty&,
diff --git a/modules/board/service.cc b/modules/board/service.cc
index 1a2284f..1da30c9 100644
--- a/modules/board/service.cc
+++ b/modules/board/service.cc
@@ -46,7 +46,10 @@
const board_OnboardTempStreamRequest& request,
ServerWriter<board_OnboardTempResponse>& writer) {
if (request.sample_interval_ms < 100) {
- writer.Finish(pw::Status::InvalidArgument());
+ if (const auto status = writer.Finish(pw::Status::InvalidArgument());
+ !status.ok()) {
+ PW_LOG_ERROR("Failed to write response: %s", status.str());
+ }
return;
}
diff --git a/modules/buttons/BUILD.bazel b/modules/buttons/BUILD.bazel
index 3bbdfbe..d8c326a 100644
--- a/modules/buttons/BUILD.bazel
+++ b/modules/buttons/BUILD.bazel
@@ -13,12 +13,6 @@
# the License.
load("@pigweed//pw_build:pigweed.bzl", "pw_cc_test")
-load(
- "@pigweed//pw_protobuf_compiler:pw_proto_library.bzl",
- "nanopb_proto_library",
- "nanopb_rpc_proto_library",
-)
-load("@rules_python//python:proto.bzl", "py_proto_library")
package(default_visibility = ["//visibility:public"])
@@ -48,6 +42,7 @@
":manager",
"//modules/worker:test_worker",
"@pigweed//pw_digital_io",
+ "@pigweed//pw_status",
"@pigweed//pw_sync:timed_thread_notification",
"@pigweed//pw_thread:sleep",
"@pigweed//pw_unit_test",
diff --git a/modules/buttons/manager.cc b/modules/buttons/manager.cc
index 22c0f31..5fe5590 100644
--- a/modules/buttons/manager.cc
+++ b/modules/buttons/manager.cc
@@ -15,6 +15,7 @@
#define PW_LOG_MODULE_NAME "BUTTONS"
#include "pw_assert/check.h"
+#include "pw_log/log.h"
#include "pw_status/try.h"
using pw::chrono::SystemClock;
@@ -77,7 +78,9 @@
void ButtonManager::SampleCallback(SystemClock::time_point now) {
PW_CHECK_NOTNULL(worker_);
worker_->RunOnce([this, now]() {
- SampleButtons(now);
+ if (const auto status = SampleButtons(now); !status.ok()) {
+ PW_LOG_ERROR("Failed to sample buttons: %s", status.str());
+ }
// Start the periodic sampling callbacks.
timer_.InvokeAfter(kSampleInterval);
});
diff --git a/modules/buttons/manager.h b/modules/buttons/manager.h
index a761e95..6ea9e4a 100644
--- a/modules/buttons/manager.h
+++ b/modules/buttons/manager.h
@@ -15,6 +15,7 @@
#include <chrono>
+#include "pw_assert/check.h"
#include "modules/pubsub/pubsub_events.h"
#include "modules/worker/worker.h"
#include "pw_chrono/system_clock.h"
@@ -61,7 +62,7 @@
class Button {
public:
- Button(pw::digital_io::DigitalIn& io) : io_(io) { io_.Enable(); };
+ Button(pw::digital_io::DigitalIn& io) : io_(io) { PW_CHECK_OK(io_.Enable()); };
pw::Result<EdgeDetector::StateChange> Sample(
pw::chrono::SystemClock::time_point now);
diff --git a/modules/buttons/manager_test.cc b/modules/buttons/manager_test.cc
index 03dc315..e4ec9e6 100644
--- a/modules/buttons/manager_test.cc
+++ b/modules/buttons/manager_test.cc
@@ -16,6 +16,8 @@
#include "modules/worker/test_worker.h"
#include "pw_digital_io/digital_io.h"
+#include "pw_status/status.h"
+#include "pw_sync/timed_thread_notification.h"
#include "pw_sync/interrupt_spin_lock.h"
#include "pw_sync/timed_thread_notification.h"
#include "pw_unit_test/framework.h"
@@ -77,10 +79,10 @@
last_event_ = {};
events_processed_ = 0;
- io_a_.SetState(State::kInactive);
- io_b_.SetState(State::kInactive);
- io_x_.SetState(State::kInactive);
- io_y_.SetState(State::kInactive);
+ ASSERT_EQ(pw::OkStatus(), io_a_.SetState(State::kInactive));
+ ASSERT_EQ(pw::OkStatus(), io_b_.SetState(State::kInactive));
+ ASSERT_EQ(pw::OkStatus(), io_x_.SetState(State::kInactive));
+ ASSERT_EQ(pw::OkStatus(), io_y_.SetState(State::kInactive));
}
/// Expects that a button was pressed.
@@ -267,28 +269,28 @@
notification_.release();
}));
- io_a_.SetState(State::kActive);
+ ASSERT_EQ(pw::OkStatus(), io_a_.SetState(State::kActive));
ASSERT_TRUE(AssertPressed<sense::ButtonA>());
- io_b_.SetState(State::kActive);
+ ASSERT_EQ(pw::OkStatus(), io_b_.SetState(State::kActive));
ASSERT_TRUE(AssertPressed<sense::ButtonB>());
- io_x_.SetState(State::kActive);
+ ASSERT_EQ(pw::OkStatus(), io_x_.SetState(State::kActive));
ASSERT_TRUE(AssertPressed<sense::ButtonX>());
- io_y_.SetState(State::kActive);
+ ASSERT_EQ(pw::OkStatus(), io_y_.SetState(State::kActive));
ASSERT_TRUE(AssertPressed<sense::ButtonY>());
- io_a_.SetState(State::kInactive);
+ ASSERT_EQ(pw::OkStatus(), io_a_.SetState(State::kInactive));
ASSERT_TRUE(AssertPressed<sense::ButtonA>(false));
- io_b_.SetState(State::kInactive);
+ ASSERT_EQ(pw::OkStatus(), io_b_.SetState(State::kInactive));
ASSERT_TRUE(AssertPressed<sense::ButtonB>(false));
- io_x_.SetState(State::kInactive);
+ ASSERT_EQ(pw::OkStatus(), io_x_.SetState(State::kInactive));
ASSERT_TRUE(AssertPressed<sense::ButtonX>(false));
- io_y_.SetState(State::kInactive);
+ ASSERT_EQ(pw::OkStatus(), io_y_.SetState(State::kInactive));
ASSERT_TRUE(AssertPressed<sense::ButtonY>(false));
worker.Stop();
diff --git a/modules/led/BUILD.bazel b/modules/led/BUILD.bazel
index 0d6b9fb..e88e1c6 100644
--- a/modules/led/BUILD.bazel
+++ b/modules/led/BUILD.bazel
@@ -20,6 +20,7 @@
hdrs = ["monochrome_led.h"],
deps = [
"//modules/pwm:digital_out",
+ "@pigweed//pw_assert:check",
"@pigweed//pw_digital_io",
],
)
@@ -42,6 +43,7 @@
"@pigweed//pw_log",
],
deps = [
+ "@pigweed//pw_assert:check",
"@pigweed//pw_chrono:system_clock",
"@pigweed//pw_containers:inline_deque",
"@pigweed//pw_digital_io",
diff --git a/modules/led/digital_io_fake.h b/modules/led/digital_io_fake.h
index 1540b0d..cd97ad2 100644
--- a/modules/led/digital_io_fake.h
+++ b/modules/led/digital_io_fake.h
@@ -13,6 +13,7 @@
// the License.
#pragma once
+#include "pw_assert/check.h"
#include "pw_chrono/system_clock.h"
#include "pw_containers/inline_deque.h"
#include "pw_digital_io/digital_io.h"
@@ -60,7 +61,7 @@
DigitalInOutFake() : DigitalInOutFake(Clock::RealClock()) {}
DigitalInOutFake(Clock& clock) : DigitalInOutFakeImpl(clock, events_) {
- SetState(State::kInactive);
+ PW_CHECK_OK(SetState(State::kInactive));
}
private:
diff --git a/modules/led/monochrome_led.cc b/modules/led/monochrome_led.cc
index 3ea1203..36cdebb 100644
--- a/modules/led/monochrome_led.cc
+++ b/modules/led/monochrome_led.cc
@@ -12,6 +12,7 @@
// License for the specific language governing permissions and limitations under
// the License.
+#include "pw_assert/check.h"
#include "modules/led/monochrome_led.h"
namespace sense {
@@ -33,12 +34,12 @@
void MonochromeLed::TurnOn() {
SetMode(Mode::kSio);
- sio_.SetState(State::kActive);
+ PW_CHECK_OK(sio_.SetState(State::kActive));
}
void MonochromeLed::TurnOff() {
SetMode(Mode::kSio);
- sio_.SetState(State::kInactive);
+ PW_CHECK_OK(sio_.SetState(State::kInactive));
}
void MonochromeLed::SetBrightness(uint16_t level) {
@@ -51,7 +52,7 @@
bool is_off =
mode_ != Mode::kSio || !result.ok() || *result == State::kInactive;
SetMode(Mode::kSio);
- sio_.SetState(is_off ? State::kActive : State::kInactive);
+ PW_CHECK_OK(sio_.SetState(is_off ? State::kActive : State::kInactive));
}
void MonochromeLed::Pulse(uint32_t interval_ms) {
@@ -78,12 +79,12 @@
}
switch (mode_) {
case Mode::kSio:
- sio_.Disable();
+ PW_CHECK_OK(sio_.Disable());
pwm_.Enable();
break;
case Mode::kPwm:
pwm_.Disable();
- sio_.Enable();
+ PW_CHECK_OK(sio_.Enable());
break;
}
mode_ = mode;
diff --git a/modules/pubsub/BUILD.bazel b/modules/pubsub/BUILD.bazel
index 5fb3264..c887735 100644
--- a/modules/pubsub/BUILD.bazel
+++ b/modules/pubsub/BUILD.bazel
@@ -30,6 +30,7 @@
hdrs = ["pubsub.h"],
deps = [
"//modules/worker",
+ "@pigweed//pw_assert:check",
"@pigweed//pw_containers:inline_deque",
"@pigweed//pw_function",
"@pigweed//pw_sync:interrupt_spin_lock",
diff --git a/modules/pubsub/service.cc b/modules/pubsub/service.cc
index 0bc3411..88a1395 100644
--- a/modules/pubsub/service.cc
+++ b/modules/pubsub/service.cc
@@ -78,8 +78,12 @@
proto.type.sense_state.alarm_active = state.alarm;
proto.type.sense_state.alarm_threshold = state.alarm_threshold;
proto.type.sense_state.aq_score = state.air_quality;
- pw::string::Copy(state.air_quality_description,
- proto.type.sense_state.aq_description);
+ if (const auto status =
+ pw::string::Copy(state.air_quality_description,
+ proto.type.sense_state.aq_description);
+ !status.ok()) {
+ PW_LOG_ERROR("Failed to copy description: %s", status.status().str());
+ }
} else if (std::holds_alternative<StateManagerControl>(event)) {
proto.which_type = pubsub_Event_state_manager_control_tag;
const auto& control = std::get<StateManagerControl>(event);
@@ -167,8 +171,10 @@
void PubSubService::Init(PubSub& pubsub) {
pubsub_ = &pubsub;
- PW_CHECK(pubsub_->Subscribe(
- [this](Event event) { stream_.Write(EventToProto(event)); }));
+ PW_CHECK(pubsub_->Subscribe([this](Event event) {
+ // Writing to an unopened stream is okay here, so we IgnoreError.
+ stream_.Write(EventToProto(event)).IgnoreError();
+ }));
}
pw::Status PubSubService::Publish(const pubsub_Event& request,
diff --git a/modules/worker/test_worker.cc b/modules/worker/test_worker.cc
index 623d83e..29224c3 100644
--- a/modules/worker/test_worker.cc
+++ b/modules/worker/test_worker.cc
@@ -26,7 +26,8 @@
}
void GenericTestWorker::RunOnce(pw::Function<void()>&& work) {
- work_queue_->PushWork(std::move(work));
+ // TODO: http://b/357935830 - Maybe don't IgnoreError?
+ work_queue_->PushWork(std::move(work)).IgnoreError();
}
GenericTestWorker::~GenericTestWorker() {
