iioservice: Support hrtimer for acpi-als
This commit adds support on acpi-als as a light sensor with hrtimer. The
changes can also be used to support further sensors that need hrtimer
for samples.
BUG=b:174544536
TEST=unit tests and run on nightfury
Change-Id: I0f0bbc2adccd4244aa93194d92da139722933755
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2574151
Commit-Queue: Cheng-Hao Yang <chenghaoyang@chromium.org>
Tested-by: Cheng-Hao Yang <chenghaoyang@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
diff --git a/iioservice/daemon/samples_handler.cc b/iioservice/daemon/samples_handler.cc
index 7130c34..93c5f9d 100644
--- a/iioservice/daemon/samples_handler.cc
+++ b/iioservice/daemon/samples_handler.cc
@@ -27,6 +27,9 @@
constexpr char kNoBatchChannels[][10] = {"timestamp", "count"};
constexpr char kHWFifoFlushPath[] = "buffer/hwfifo_flush";
+constexpr double kAcpiAlsMinFrequency = 0.1;
+constexpr double kAcpiAlsMaxFrequency = 2.0;
+
} // namespace
// static
@@ -56,7 +59,7 @@
}
// static
-SamplesHandler::ScopedSamplesHandler SamplesHandler::CreateWithFifo(
+SamplesHandler::ScopedSamplesHandler SamplesHandler::Create(
scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
libmems::IioDevice* iio_device,
@@ -68,46 +71,16 @@
return handler;
double min_freq, max_freq;
- if (!iio_device->GetMinMaxFrequency(&min_freq, &max_freq))
+ if (strcmp(iio_device->GetName(), "acpi-als") == 0) {
+ min_freq = kAcpiAlsMinFrequency;
+ max_freq = kAcpiAlsMaxFrequency;
+ } else if (!iio_device->GetMinMaxFrequency(&min_freq, &max_freq)) {
return handler;
-
- handler.reset(new SamplesHandler(
- std::move(ipc_task_runner), std::move(sample_task_runner), iio_device,
- min_freq, max_freq, std::move(on_sample_updated_callback),
- std::move(on_error_occurred_callback)));
- return handler;
-}
-
-// static
-SamplesHandler::ScopedSamplesHandler SamplesHandler::CreateWithoutFifo(
- scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
- libmems::IioContext* iio_context,
- libmems::IioDevice* iio_device,
- OnSampleUpdatedCallback on_sample_updated_callback,
- OnErrorOccurredCallback on_error_occurred_callback) {
- ScopedSamplesHandler handler(nullptr, SamplesHandlerDeleter);
-
- if (!DisableBufferAndEnableChannels(iio_device))
- return handler;
-
- double min_freq, max_freq;
- if (!iio_device->GetMinMaxFrequency(&min_freq, &max_freq))
- return handler;
-
- auto trigger_device = iio_device->GetTrigger();
- if (!trigger_device) {
- trigger_device = iio_context->GetTriggerById(iio_device->GetId() + 1);
- if (!trigger_device) {
- LOGF(ERROR) << "Failed to find trigger with id: " << iio_device->GetId();
-
- return handler;
- }
}
handler.reset(new SamplesHandler(
std::move(ipc_task_runner), std::move(sample_task_runner), iio_device,
- trigger_device, min_freq, max_freq, std::move(on_sample_updated_callback),
+ min_freq, max_freq, std::move(on_sample_updated_callback),
std::move(on_error_occurred_callback)));
return handler;
}
@@ -181,40 +154,7 @@
OnErrorOccurredCallback on_error_occurred_callback)
: ipc_task_runner_(std::move(ipc_task_runner)),
sample_task_runner_(std::move(sample_task_runner)),
- use_fifo_(true),
iio_device_(iio_device),
- trigger_device_(nullptr),
- dev_min_frequency_(min_freq),
- dev_max_frequency_(max_freq),
- on_sample_updated_callback_(std::move(on_sample_updated_callback)),
- on_error_occurred_callback_(std::move(on_error_occurred_callback)) {
- DCHECK_GE(dev_max_frequency_, dev_min_frequency_);
-
- auto channels = iio_device_->GetAllChannels();
- for (size_t i = 0; i < channels.size(); ++i) {
- for (size_t j = 0; j < base::size(kNoBatchChannels); ++j) {
- if (strcmp(channels[i]->GetId(), kNoBatchChannels[j]) == 0) {
- no_batch_chn_indices.emplace(i);
- break;
- }
- }
- }
-}
-
-SamplesHandler::SamplesHandler(
- scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
- libmems::IioDevice* iio_device,
- libmems::IioDevice* trigger_device,
- double min_freq,
- double max_freq,
- OnSampleUpdatedCallback on_sample_updated_callback,
- OnErrorOccurredCallback on_error_occurred_callback)
- : ipc_task_runner_(std::move(ipc_task_runner)),
- sample_task_runner_(std::move(sample_task_runner)),
- use_fifo_(false),
- iio_device_(iio_device),
- trigger_device_(trigger_device),
dev_min_frequency_(min_freq),
dev_max_frequency_(max_freq),
on_sample_updated_callback_(std::move(on_sample_updated_callback)),
@@ -236,8 +176,10 @@
DCHECK(sample_task_runner_->BelongsToCurrentThread());
// Flush the old samples in EC FIFO.
- if (!iio_device_->WriteStringAttribute(kHWFifoFlushPath, "1\n"))
+ if (!iio_device_->GetTrigger() &&
+ !iio_device_->WriteStringAttribute(kHWFifoFlushPath, "1\n")) {
LOGF(ERROR) << "Failed to flush the old samples in EC FIFO";
+ }
auto fd = iio_device_->GetBufferFd();
if (!fd.has_value()) {
@@ -472,22 +414,20 @@
return true;
requested_frequency_ = frequency;
- if (!iio_device_->WriteDoubleAttribute(libmems::kSamplingFrequencyAttr,
- frequency)) {
- LOGF(ERROR) << "Failed to set frequency";
- if (use_fifo_) // ignore this error if no fifo
+ if (!iio_device_->GetTrigger()) {
+ if (!iio_device_->WriteDoubleAttribute(libmems::kSamplingFrequencyAttr,
+ frequency)) {
+ LOGF(ERROR) << "Failed to set frequency";
+ }
+
+ auto freq_opt =
+ iio_device_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr);
+ if (!freq_opt.has_value()) {
+ LOGF(ERROR) << "Failed to get frequency";
return false;
- }
+ }
+ dev_frequency_ = freq_opt.value();
- auto freq_opt =
- iio_device_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr);
- if (!freq_opt.has_value()) {
- LOGF(ERROR) << "Failed to get frequency";
- return false;
- }
- dev_frequency_ = freq_opt.value();
-
- if (use_fifo_) {
if (dev_frequency_ < libmems::kFrequencyEpsilon)
return true;
@@ -500,14 +440,19 @@
return true;
}
- // no fifo
- DCHECK(trigger_device_);
-
- if (!trigger_device_->WriteDoubleAttribute(libmems::kSamplingFrequencyAttr,
- frequency)) {
+ iio_device_->WriteDoubleAttribute(libmems::kSamplingFrequencyAttr, frequency);
+ if (!iio_device_->GetTrigger()->WriteDoubleAttribute(
+ libmems::kSamplingFrequencyAttr, frequency)) {
LOGF(ERROR) << "Failed to set trigger's frequency";
return false;
}
+ auto freq_opt = iio_device_->GetTrigger()->ReadDoubleAttribute(
+ libmems::kSamplingFrequencyAttr);
+ if (!freq_opt.has_value()) {
+ LOGF(ERROR) << "Failed to get frequency";
+ return false;
+ }
+ dev_frequency_ = freq_opt.value();
return true;
}
diff --git a/iioservice/daemon/samples_handler.h b/iioservice/daemon/samples_handler.h
index c1ff76f..6ff3400 100644
--- a/iioservice/daemon/samples_handler.h
+++ b/iioservice/daemon/samples_handler.h
@@ -41,21 +41,12 @@
mojo::ReceiverId, cros::mojom::ObserverErrorType)>;
static bool DisableBufferAndEnableChannels(libmems::IioDevice* iio_device);
- // use fifo
- static ScopedSamplesHandler CreateWithFifo(
+ static ScopedSamplesHandler Create(
scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
libmems::IioDevice* iio_device,
OnSampleUpdatedCallback on_sample_updated_callback,
OnErrorOccurredCallback on_error_occurred_callback);
- // no fifo
- static ScopedSamplesHandler CreateWithoutFifo(
- scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
- libmems::IioContext* iio_context,
- libmems::IioDevice* iio_device,
- OnSampleUpdatedCallback on_sample_updated_callback,
- OnErrorOccurredCallback on_error_occurred_callback);
virtual ~SamplesHandler();
@@ -99,15 +90,6 @@
double max_freq,
OnSampleUpdatedCallback on_sample_updated_callback,
OnErrorOccurredCallback on_error_occurred_callback);
- // no fifo
- SamplesHandler(scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner,
- libmems::IioDevice* iio_device,
- libmems::IioDevice* trigger_device,
- double in_freq,
- double ax_freq,
- OnSampleUpdatedCallback on_sample_updated_callback,
- OnErrorOccurredCallback on_error_occurred_callback);
void SetSampleWatcherOnThread();
void StartAcceptingSamples(
@@ -151,9 +133,7 @@
scoped_refptr<base::SequencedTaskRunner> ipc_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> sample_task_runner_;
- bool use_fifo_ = true;
libmems::IioDevice* iio_device_;
- libmems::IioDevice* trigger_device_ = nullptr;
// Clients that either have invalid frequency or no enabled channels.
std::set<ClientData*> inactive_clients_;
diff --git a/iioservice/daemon/samples_handler_test.cc b/iioservice/daemon/samples_handler_test.cc
index 35eef2e..1cfebe2 100644
--- a/iioservice/daemon/samples_handler_test.cc
+++ b/iioservice/daemon/samples_handler_test.cc
@@ -32,6 +32,9 @@
constexpr double kFooFrequency = 20.0;
constexpr int kNumFailures = 10;
+constexpr char kFakeTriggerName[] = "FakeTrigger";
+constexpr int kFakeTriggerId = 1;
+
double FixFrequency(double frequency) {
if (frequency < libmems::kFrequencyEpsilon)
return 0.0;
@@ -51,7 +54,9 @@
libmems::IioDevice::IioSample sample) {
CHECK(
task_environment_.GetMainThreadTaskRunner()->BelongsToCurrentThread());
- CHECK_LT(id, observers_.size());
+
+ if (id >= observers_.size())
+ return;
observers_[id]->OnSampleUpdated(std::move(sample));
}
@@ -67,9 +72,15 @@
observers_[id]->OnErrorOccurred(type);
}
- void SetUpBase() {
+ void SetUpBase(bool with_trigger) {
device_ = std::make_unique<libmems::fakes::FakeIioDevice>(
nullptr, fakes::kAccelDeviceName, fakes::kAccelDeviceId);
+ if (with_trigger) {
+ trigger_ = std::make_unique<libmems::fakes::FakeIioDevice>(
+ nullptr, kFakeTriggerName, kFakeTriggerId);
+ device_->SetTrigger(trigger_.get());
+ }
+
EXPECT_TRUE(
device_->WriteStringAttribute(libmems::kSamplingFrequencyAvailable,
fakes::kFakeSamplingFrequencyAvailable));
@@ -82,7 +93,7 @@
EXPECT_TRUE(
device_->WriteDoubleAttribute(libmems::kSamplingFrequencyAttr, 0.0));
- handler_ = fakes::FakeSamplesHandler::CreateWithFifo(
+ handler_ = fakes::FakeSamplesHandler::Create(
task_environment_.GetMainThreadTaskRunner(),
task_environment_.GetMainThreadTaskRunner(), device_.get(),
base::BindRepeating(&SamplesHandlerTestBase::OnSampleUpdatedCallback,
@@ -108,6 +119,7 @@
base::test::TaskEnvironment::MainThreadType::IO};
std::unique_ptr<libmems::fakes::FakeIioDevice> device_;
+ std::unique_ptr<libmems::fakes::FakeIioDevice> trigger_;
fakes::FakeSamplesHandler::ScopedFakeSamplesHandler handler_ = {
nullptr, SamplesHandler::SamplesHandlerDeleter};
@@ -118,7 +130,7 @@
class SamplesHandlerTest : public ::testing::Test,
public SamplesHandlerTestBase {
protected:
- void SetUp() override { SetUpBase(); }
+ void SetUp() override { SetUpBase(/*with_trigger=*/false); }
void TearDown() override { TearDownBase(); }
};
@@ -233,7 +245,7 @@
: public ::testing::TestWithParam<std::vector<std::pair<double, double>>>,
public SamplesHandlerTestBase {
protected:
- void SetUp() override { SetUpBase(); }
+ void SetUp() override { SetUpBase(/*with_trigger=*/false); }
void TearDown() override { TearDownBase(); }
};
@@ -364,6 +376,13 @@
base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(device_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr)
+ .value_or(-1),
+ max_freq);
+ EXPECT_EQ(
+ device_->ReadDoubleAttribute(libmems::kHWFifoTimeoutAttr).value_or(-1),
+ 1.0 / max_freq);
+
device_->SetPauseCallbackAtKthSamples(
fakes::kPauseIndex,
base::BindOnce(
@@ -400,6 +419,13 @@
// Wait until all observers receive all samples
base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(device_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr)
+ .value_or(-1),
+ max_freq2);
+ EXPECT_EQ(
+ device_->ReadDoubleAttribute(libmems::kHWFifoTimeoutAttr).value_or(-1),
+ 1.0 / max_freq2);
+
for (const auto& observer : observers_)
EXPECT_TRUE(observer->FinishedObserving());
@@ -429,6 +455,36 @@
std::vector<std::pair<double, double>>{{20.0, 30.0},
{10.0, 10.0}}));
+class SamplesHandlerWithTriggerTest : public ::testing::Test,
+ public SamplesHandlerTestBase {
+ protected:
+ void SetUp() override { SetUpBase(/*with_trigger=*/true); }
+
+ void TearDown() override { TearDownBase(); }
+};
+
+TEST_F(SamplesHandlerWithTriggerTest, CheckFrequenciesSet) {
+ ClientData client_data;
+
+ double frequency = kMaxFrequency;
+
+ client_data.id = 0;
+ client_data.iio_device = device_.get();
+ client_data.enabled_chn_indices.emplace(0); // accel_x
+ client_data.frequency = frequency;
+
+ handler_->AddClient(&client_data);
+
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_EQ(device_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr)
+ .value_or(-1),
+ frequency);
+ EXPECT_EQ(trigger_->ReadDoubleAttribute(libmems::kSamplingFrequencyAttr)
+ .value_or(-1),
+ frequency);
+}
+
} // namespace
} // namespace iioservice
diff --git a/iioservice/daemon/sensor_device_impl.cc b/iioservice/daemon/sensor_device_impl.cc
index 6fa4a68..6484216 100644
--- a/iioservice/daemon/sensor_device_impl.cc
+++ b/iioservice/daemon/sensor_device_impl.cc
@@ -46,9 +46,8 @@
return device;
}
- // TODO(chenghaoyang): Check how to detect it's Samus, which doesn't use fifo.
device.reset(new SensorDeviceImpl(std::move(ipc_task_runner), context,
- std::move(thread), true));
+ std::move(thread)));
return device;
}
@@ -265,12 +264,10 @@
SensorDeviceImpl::SensorDeviceImpl(
scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
libmems::IioContext* context,
- std::unique_ptr<base::Thread> thread,
- bool use_fifo)
+ std::unique_ptr<base::Thread> thread)
: ipc_task_runner_(std::move(ipc_task_runner)),
context_(std::move(context)),
- sample_thread_(std::move(thread)),
- use_fifo_(use_fifo) {
+ sample_thread_(std::move(thread)) {
DCHECK(ipc_task_runner_->RunsTasksInCurrentSequence());
receiver_set_.set_disconnect_handler(base::BindRepeating(
@@ -353,15 +350,9 @@
auto error_cb = base::BindRepeating(
&SensorDeviceImpl::OnErrorOccurredCallback, weak_factory_.GetWeakPtr());
- if (use_fifo_) {
- handler = SamplesHandler::CreateWithFifo(
- ipc_task_runner_, sample_thread_->task_runner(), iio_device,
- std::move(sample_cb), std::move(error_cb));
- } else {
- handler = SamplesHandler::CreateWithoutFifo(
- ipc_task_runner_, sample_thread_->task_runner(), context_, iio_device,
- std::move(sample_cb), std::move(error_cb));
- }
+ handler = SamplesHandler::Create(ipc_task_runner_,
+ sample_thread_->task_runner(), iio_device,
+ std::move(sample_cb), std::move(error_cb));
if (!handler) {
LOGF(ERROR) << "Failed to create the samples handler for device: "
diff --git a/iioservice/daemon/sensor_device_impl.h b/iioservice/daemon/sensor_device_impl.h
index ccdef98..69dec3e 100644
--- a/iioservice/daemon/sensor_device_impl.h
+++ b/iioservice/daemon/sensor_device_impl.h
@@ -63,8 +63,7 @@
private:
SensorDeviceImpl(scoped_refptr<base::SequencedTaskRunner> ipc_task_runner,
libmems::IioContext* context,
- std::unique_ptr<base::Thread> thread,
- bool use_fifo);
+ std::unique_ptr<base::Thread> thread);
void AddReceiverOnThread(
int32_t iio_device_id,
@@ -87,7 +86,6 @@
libmems::IioContext* context_; // non-owned
mojo::ReceiverSet<cros::mojom::SensorDevice> receiver_set_;
std::unique_ptr<base::Thread> sample_thread_;
- bool use_fifo_ = true;
std::map<mojo::ReceiverId, ClientData> clients_;
diff --git a/iioservice/daemon/sensor_service_impl.cc b/iioservice/daemon/sensor_service_impl.cc
index cc8a238..446d89c 100644
--- a/iioservice/daemon/sensor_service_impl.cc
+++ b/iioservice/daemon/sensor_service_impl.cc
@@ -201,26 +201,43 @@
}
}
+void SensorServiceImpl::FailedToLoadDevice(libmems::IioDevice* device) {
+ DCHECK(ipc_task_runner_->RunsTasksInCurrentSequence());
+
+ const int32_t id = device->GetId();
+ if (++iio_device_permission_trials_[id] >=
+ kNumFailedPermTrialsBeforeGivingUp) {
+ LOGF(ERROR) << "Too many failed permission trials. Giving up on device: "
+ << id;
+ return;
+ }
+
+ LOGF(WARNING) << "Permissions and ownerships may not be set yet for device: "
+ << id;
+ ipc_task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&SensorServiceImpl::AddDevice, weak_factory_.GetWeakPtr(),
+ device),
+ base::TimeDelta::FromMilliseconds(kPermTrialDelayInMilliseconds));
+}
+
void SensorServiceImpl::AddDevice(libmems::IioDevice* device) {
DCHECK(ipc_task_runner_->RunsTasksInCurrentSequence());
const int32_t id = device->GetId();
if (!device->DisableBuffer()) {
- if (++iio_device_permission_trials_[id] >=
- kNumFailedPermTrialsBeforeGivingUp) {
- LOGF(ERROR) << "Too many failed permission trials. Giving up on device: "
- << id;
+ FailedToLoadDevice(device);
+ return;
+ }
+
+ if (strcmp(device->GetName(), "acpi-als") == 0 && !device->GetTrigger()) {
+ // Reloads context for the hrtimer, as it'd be added after mems_setup.
+ context_->Reload();
+
+ if (!device->GetTrigger()) {
+ FailedToLoadDevice(device);
return;
}
-
- LOGF(WARNING)
- << "Permissions and ownerships may not be set yet for device: " << id;
- ipc_task_runner_->PostDelayedTask(
- FROM_HERE,
- base::BindOnce(&SensorServiceImpl::AddDevice,
- weak_factory_.GetWeakPtr(), device),
- base::TimeDelta::FromMilliseconds(kPermTrialDelayInMilliseconds));
- return;
}
std::vector<cros::mojom::DeviceType> types;
diff --git a/iioservice/daemon/sensor_service_impl.h b/iioservice/daemon/sensor_service_impl.h
index 9955cae..b9dd7ca 100644
--- a/iioservice/daemon/sensor_service_impl.h
+++ b/iioservice/daemon/sensor_service_impl.h
@@ -62,6 +62,7 @@
private:
static const uint32_t kNumFailedPermTrialsBeforeGivingUp = 10;
+ void FailedToLoadDevice(libmems::IioDevice* device);
void AddDevice(libmems::IioDevice* device);
scoped_refptr<base::SequencedTaskRunner> ipc_task_runner_;
diff --git a/iioservice/daemon/test_fakes.cc b/iioservice/daemon/test_fakes.cc
index 1204ad6..6ac6805 100644
--- a/iioservice/daemon/test_fakes.cc
+++ b/iioservice/daemon/test_fakes.cc
@@ -30,7 +30,7 @@
} // namespace
// static
-FakeSamplesHandler::ScopedFakeSamplesHandler FakeSamplesHandler::CreateWithFifo(
+FakeSamplesHandler::ScopedFakeSamplesHandler FakeSamplesHandler::Create(
scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
libmems::fakes::FakeIioDevice* fake_iio_device,
diff --git a/iioservice/daemon/test_fakes.h b/iioservice/daemon/test_fakes.h
index c77ba8b..480800c 100644
--- a/iioservice/daemon/test_fakes.h
+++ b/iioservice/daemon/test_fakes.h
@@ -37,7 +37,7 @@
using ScopedFakeSamplesHandler =
std::unique_ptr<FakeSamplesHandler, decltype(&SamplesHandlerDeleter)>;
- static ScopedFakeSamplesHandler CreateWithFifo(
+ static ScopedFakeSamplesHandler Create(
scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
libmems::fakes::FakeIioDevice* fake_iio_device,
