Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: sensor: pinnacle: add driver for Cirque Pinnacle GlidePoint trackpads #69431

Closed

Conversation

akscram
Copy link
Contributor

@akscram akscram commented Feb 25, 2024

Introduce a sensor driver for Cirque Pinnacle GlidePoint circle trackpads connected via SPI, such as TM035035 and similar.

TODOs:

  • Sample which demonstrates a use of the sensor
  • Relative mode

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Sensors Sensors labels Feb 25, 2024
Copy link

Hello @akscram, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@akscram akscram changed the title drivers: sensor: pinnacle: add driver for trackpad drivers: sensor: pinnacle: add driver for Cirque Pinnacle GlidePoint trackpads Feb 25, 2024
@akscram akscram force-pushed the sensor-cirque-pinnacle branch from 872f136 to c1907e8 Compare February 25, 2024 22:36
@zephyrbot zephyrbot added the area: Samples Samples label Feb 25, 2024
@zephyrbot zephyrbot requested review from kartben and nashif February 25, 2024 22:36
@kartben
Copy link
Collaborator

kartben commented Feb 25, 2024

Thanks for the PR! Please look at implementing this as an input driver (see drivers/input) as it doesn't really sound right to implement this as a sensor (the fact that you had to touch the sensor.h file is a good indicator)

Introduce a sensor driver for Cirque Pinnacle GlidePoint circle
trackpads connected via SPI, such as TM035035 and similar.

Signed-off-by: Ilia Kharin <[email protected]>
@akscram akscram force-pushed the sensor-cirque-pinnacle branch from c1907e8 to 3dbace3 Compare February 25, 2024 23:29
@akscram
Copy link
Contributor Author

akscram commented Feb 25, 2024

@kartben, thank you for an early feedback. I changed sensor.h in order to add a channel which provides absolute positions for three coordinates at once because at the moment there are three separate channels only for three relative coordinates. I'll take a look at how to implement the input driver interface for the trackpad. If I can get it to work, should I post it in a separate PR or just updating this one will do too?

@akscram
Copy link
Contributor Author

akscram commented Feb 26, 2024

@kartben, as you suggested, I implemented the driver as an input device: #69438. Please, let me know that this is a right direction and on which one I should continue working.

@akscram
Copy link
Contributor Author

akscram commented Feb 26, 2024

I decided to compare these two approaches and it seems that the approach with the sensor is much closer to real-time processing and much more consistent to 10 ms which is a period when the device triggers a data-ready interrupt:

*** Booting Zephyr OS build v3.6.0-rc2-175-g3dbace35bd41 ***
[00:00:10.474,182] <inf> main: 1175:869:16
[00:00:10.483,245] <inf> main: 1175:869:15
[00:00:10.492,950] <inf> main: 1175:869:16
[00:00:10.503,173] <inf> main: 1175:869:16
[00:00:10.512,329] <inf> main: 1175:869:16
[00:00:10.521,636] <inf> main: 1176:870:16
[00:00:10.532,196] <inf> main: 1178:870:16
[00:00:10.541,900] <inf> main: 1181:870:16
[00:00:10.551,055] <inf> main: 1184:870:16
[00:00:10.561,004] <inf> main: 1187:871:16
[00:00:10.570,404] <inf> main: 1189:873:16
[00:00:10.580,383] <inf> main: 1191:874:17
[00:00:10.590,057] <inf> main: 1193:875:17
[00:00:10.599,121] <inf> main: 1195:875:16
[00:00:10.609,405] <inf> main: 1196:876:15
[00:00:10.619,628] <inf> main: 1197:877:16
[00:00:10.628,509] <inf> main: 1198:877:16
[00:00:10.638,214] <inf> main: 1198:878:17
[00:00:10.648,162] <inf> main: 1198:879:16
[00:00:10.657,806] <inf> main: 1198:880:16
[00:00:10.667,236] <inf> main: 1198:881:16
[00:00:10.676,849] <inf> main: 1197:882:16
[00:00:10.686,859] <inf> main: 1194:882:14
[00:00:10.696,807] <inf> main: 1191:882:8
[00:00:10.704,498] <inf> main: 0:0:0
[00:00:10.714,416] <inf> main: 0:0:0
[00:00:10.723,846] <inf> main: 0:0:0
[00:00:10.733,551] <inf> main: 0:0:0
[00:00:10.743,225] <inf> main: 0:0:0
[00:00:10.752,899] <inf> main: 0:0:0
[00:00:10.762,847] <inf> main: 0:0:0
[00:00:10.772,796] <inf> main: 0:0:0
[00:00:10.782,196] <inf> main: 0:0:0
[00:00:10.791,625] <inf> main: 0:0:0
[00:00:10.801,300] <inf> main: 0:0:0
[00:00:10.810,974] <inf> main: 0:0:0
[00:00:10.820,922] <inf> main: 0:0:0
[00:00:10.830,596] <inf> main: 0:0:0
[00:00:10.840,026] <inf> main: 0:0:0
[00:00:10.849,975] <inf> main: 0:0:0
[00:00:10.859,649] <inf> main: 0:0:0
[00:00:10.869,079] <inf> main: 0:0:0
[00:00:10.878,997] <inf> main: 0:0:0
[00:00:10.888,702] <inf> main: 0:0:0

In comparison to the approach with the input driver:

*** Booting Zephyr OS build v3.6.0-rc2-175-g667ac664b5d8 ***
[00:00:10.890,594] <inf> main: 564:542:16
[00:00:10.900,238] <inf> main: 564:542:15
[00:00:10.909,637] <inf> main: 564:542:16
[00:00:10.919,555] <inf> main: 564:542:15
[00:00:10.929,260] <inf> main: 564:542:16
[00:00:10.938,293] <inf> main: 565:542:16
[00:00:10.948,577] <inf> main: 566:543:16
[00:00:10.958,801] <inf> main: 567:544:17
[00:00:10.967,681] <inf> main: 569:545:16
[00:00:10.977,355] <inf> main: 571:546:17
[00:00:11.023,895] <inf> main: 573:548:16
[00:00:11.023,986] <inf> main: 574:549:16
[00:00:11.024,017] <inf> main: 576:550:17
[00:00:11.024,078] <inf> main: 577:551:16
[00:00:11.026,062] <inf> main: 580:552:17
[00:00:11.036,010] <inf> main: 581:552:17
[00:00:11.045,440] <inf> main: 582:553:17
[00:00:11.054,840] <inf> main: 582:553:16
[00:00:11.064,514] <inf> main: 582:553:16
[00:00:11.074,737] <inf> main: 583:553:17
[00:00:11.121,398] <inf> main: 583:553:16
[00:00:11.121,459] <inf> main: 583:553:16
[00:00:11.121,520] <inf> main: 583:553:9
[00:00:11.121,582] <inf> main: 0:0:0
[00:00:11.121,612] <inf> main: 0:0:0
[00:00:11.131,072] <inf> main: 0:0:0
[00:00:11.140,502] <inf> main: 0:0:0
[00:00:11.150,451] <inf> main: 0:0:0
[00:00:11.160,125] <inf> main: 0:0:0
[00:00:11.169,525] <inf> main: 0:0:0
[00:00:11.212,707] <inf> main: 0:0:0
[00:00:11.212,768] <inf> main: 0:0:0
[00:00:11.212,829] <inf> main: 0:0:0
[00:00:11.212,860] <inf> main: 0:0:0
[00:00:11.218,200] <inf> main: 0:0:0
[00:00:11.227,600] <inf> main: 0:0:0
[00:00:11.237,304] <inf> main: 0:0:0
[00:00:11.246,978] <inf> main: 0:0:0
[00:00:11.256,652] <inf> main: 0:0:0
[00:00:11.266,601] <inf> main: 0:0:0
[00:00:11.308,288] <inf> main: 0:0:0
[00:00:11.308,349] <inf> main: 0:0:0
[00:00:11.308,410] <inf> main: 0:0:0

I'm wondering whether the message queue used in the input driver subsystem is a root cause of this.

@akscram
Copy link
Contributor Author

akscram commented Feb 26, 2024

I realized that the priority of the input subsystem thread by default is the lowest one and the priority of the driver thread is set to 10. When I change the priority of the first one, I'm getting similar results where intervals between samples are close to 10 ms:

*** Booting Zephyr OS build v3.6.0-rc2-175-g667ac664b5d8 ***
[00:00:09.059,692] <inf> main: 589:457:15
[00:00:09.069,305] <inf> main: 589:457:16
[00:00:09.079,193] <inf> main: 589:457:16
[00:00:09.088,928] <inf> main: 589:457:17
[00:00:09.098,632] <inf> main: 590:458:17
[00:00:09.108,306] <inf> main: 593:461:16
[00:00:09.117,980] <inf> main: 597:464:17
[00:00:09.127,410] <inf> main: 598:466:17
[00:00:09.137,359] <inf> main: 600:469:17
[00:00:09.146,789] <inf> main: 602:473:17
[00:00:09.156,372] <inf> main: 602:476:17
[00:00:09.166,656] <inf> main: 603:478:17
[00:00:09.176,086] <inf> main: 604:480:17
[00:00:09.185,485] <inf> main: 605:480:17
[00:00:09.195,709] <inf> main: 605:481:16
[00:00:09.204,864] <inf> main: 606:482:17
[00:00:09.214,569] <inf> main: 606:483:17
[00:00:09.224,517] <inf> main: 606:484:17
[00:00:09.233,856] <inf> main: 606:485:17
[00:00:09.243,865] <inf> main: 606:485:17
[00:00:09.253,814] <inf> main: 607:486:17
[00:00:09.262,969] <inf> main: 608:487:17
[00:00:09.272,918] <inf> main: 608:488:16
[00:00:09.282,623] <inf> main: 609:488:17
[00:00:09.292,022] <inf> main: 609:489:17
[00:00:09.301,696] <inf> main: 609:489:16
[00:00:09.311,553] <inf> main: 609:489:16
[00:00:09.321,319] <inf> main: 609:489:17
[00:00:09.331,024] <inf> main: 609:489:17
[00:00:09.340,698] <inf> main: 609:489:17
[00:00:09.350,341] <inf> main: 609:489:17
[00:00:09.359,771] <inf> main: 608:488:16
[00:00:09.369,720] <inf> main: 608:488:16
[00:00:09.379,119] <inf> main: 607:487:16
[00:00:09.388,732] <inf> main: 606:485:14
[00:00:09.399,078] <inf> main: 604:482:11
[00:00:09.408,203] <inf> main: 600:477:5
[00:00:09.416,412] <inf> main: 0:0:0
[00:00:09.426,086] <inf> main: 0:0:0
[00:00:09.435,760] <inf> main: 0:0:0
[00:00:09.445,709] <inf> main: 0:0:0
[00:00:09.455,688] <inf> main: 0:0:0
[00:00:09.465,087] <inf> main: 0:0:0
[00:00:09.474,487] <inf> main: 0:0:0
[00:00:09.484,161] <inf> main: 0:0:0
[00:00:09.493,865] <inf> main: 0:0:0
[00:00:09.503,814] <inf> main: 0:0:0
[00:00:09.513,488] <inf> main: 0:0:0
[00:00:09.522,888] <inf> main: 0:0:0
[00:00:09.532,836] <inf> main: 0:0:0
[00:00:09.542,510] <inf> main: 0:0:0
[00:00:09.551,940] <inf> main: 0:0:0
[00:00:09.561,889] <inf> main: 0:0:0
[00:00:09.571,563] <inf> main: 0:0:0
[00:00:09.580,963] <inf> main: 0:0:0
[00:00:09.590,667] <inf> main: 0:0:0
[00:00:09.600,341] <inf> main: 0:0:0

@fabiobaltieri
Copy link
Member

This is certainly not going in as a sensor device, feel free to go ahead on the input version of the PR. Not sure what the latency deal was but it's unlikely to be related to the input subsystem, you can even set it up in synchronous mode and the queue is gone.

@akscram
Copy link
Contributor Author

akscram commented Feb 27, 2024

@fabiobaltieri, thank you for confirming that it should be implemented as an input driver.

@akscram
Copy link
Contributor Author

akscram commented Feb 27, 2024

It was decided to go with implementing the input driver instead #69438

@akscram akscram closed this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Samples Samples area: Sensors Sensors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants