Touch Panel HIL and ft6x06 driver #1986
Conversation
alexandruradovici
force-pushed
the
hil_touch
branch
from
313fa3e
to
8d456a5
Compare
| gesture_callback: Option<Callback>, | ||
| multi_touch_callback: Option<Callback>, | ||
| events_buffer: Option<AppSlice<Shared, u8>>, | ||
| ack: bool, |
There was a problem hiding this comment.
This avoids a read write race. Apps will get the scheduled callback, but they might be in the middle of it when the touch screen sends a new set of touches. The data would be overwritten by the capsule right when the app reads it. To avoid this, I will add a command so that apps can ack that they have read the data.
Am I maybe wrong about how callbacks work and the ack is not necessary?
There was a problem hiding this comment.
No you're right, that is something that has to be considered.
| /// Returns the number of concurently supported touches | ||
| /// This function must be called in the same interrupt | ||
| /// as the event, otherwise data might not be available. | ||
| fn get_num_touches(&self) -> usize; |
There was a problem hiding this comment.
I'm not sure I follow this API. Will the number of concurrent touches supported change? What is the event? Doesn't the event report how many touches there were?
There was a problem hiding this comment.
Each touch screen might support a different number of maximum touches, I expect this function to be constant.
There was a problem hiding this comment.
This is not necessarily a suggestion, but the way we handle something similar to this in the time HIL is by using an associated type to denote the clock frequency. The associated type itself has only a static method (i.e., one that doesn't take a self) and therefore, presumably, is easier for the compiler to resolve statically and inline the constant.
But... I don't know that we ever evaluated that it actually makes a difference, and I don't know if others agree that it's a better interface than just having an instance method in the trait.
There was a problem hiding this comment.
I get your point, but it does add some complexity to the code and seems to make it more difficult to read.
| gesture_callback: Option<Callback>, | ||
| multi_touch_callback: Option<Callback>, | ||
| events_buffer: Option<AppSlice<Shared, u8>>, | ||
| ack: bool, |
There was a problem hiding this comment.
No you're right, that is something that has to be considered.
kernel/src/hil/touch.rs
Outdated
| pub id: usize, | ||
|
|
||
| /// touch area | ||
| pub area: Option<usize>, |
There was a problem hiding this comment.
I think we should just define these, since these are features on modern touchscreens.
alexandruradovici
force-pushed
the
hil_touch
branch
from
3db41c4
to
2020106
Compare
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Pat Pannuto <pat.pannuto@gmail.com>
alexandruradovici
force-pushed
the
hil_touch
branch
from
d176187
to
55d733f
Compare
|
bors r+ |
Pull Request Overview
This pull request adds:
Testing Strategy
This pull request was tested using the STM32F412g Discovery Kit.
TODO or Help Wanted
This pull request still needs some feedback.
Even tough the documentation for ft6x06 states that gestures should be recognized, my device does not seem to recognize them. I might be doing something wrong, any ideas would be helpful. The test setup is https://github.com/UPBIoT/tock/tree/touch.
Documentation Updated
Formatting
make prepush.