search

hadess / iio-sensor-proxy

IIO accelerometer sensor to input device proxy
196 stars 63 forks source link

Add support for ACCEL_LOCATION udev property to deal with 2 sensors #262

Closed ljmf00 closed 5 years ago

ljmf00 commented 5 years ago

This pull request fixes #166 .

As referred to in this kernel patch, ACCEL_LOCATION property should be added to udev.

Related issues and pull requests:

ljmf00 commented 5 years ago

@hadess I have been busy on some school things, but I hope everything is fixed now. Otherwise, please let me know!

ljmf00 commented 5 years ago

The only thing that's missing would be a test for the parsing of the location property, which you can add in a separate commit. You can use test-mount-matrix.c as an example of how to write it.

Sure!

Let me know if you run into any hurdles. I can always take over if you don't have the time to finish this up, as the systemd changes have landed now.

I have some time now, and should be enough to complete the task.

ljmf00 commented 5 years ago

All indicated typos are fixed. I added a test case for accel location property as you mentioned. Please let me know if there's any issue.

ljmf00 commented 5 years ago

I just changed the first commit name to be the same subject, accel-location :)

hadess commented 5 years ago

accel-location: ignore 'base' location property for accel sensors

This doesn't really make sense. We ignore sensors, not properties, so I'd prefer something like:

accel-location: Ignore accel sensors with 'base' location

As you'll be making this small change, can you also remove the Signed-off-by from the commit messages at the same time?

Looks ready to commit after that!

ljmf00 commented 5 years ago

This doesn't really make sense. We ignore sensors, not properties, so I'd prefer something like:

accel-location: Ignore accel sensors with 'base' location

As you'll be making this small change, can you also remove the Signed-off-by from the commit messages at the same time?

Done!

Looks ready to commit after that!

Then, seems ready to merge :D

hadess commented 5 years ago

Then, seems ready to merge :D

And it was indeed :) Thanks!