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

drv: ft8xx: support multiple instances #83851

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hubertmis
Copy link
Member

Improve the FT8xx display driver to support multiple driver instances.

The ft8xx_reference_api.h still follows the FT8xx programming guides, limiting the usage of the driver to one instance.

@hubertmis
Copy link
Member Author

@kartben, what should I do to receive any feedback on this PR?

@kartben
Copy link
Collaborator

kartben commented Jan 24, 2025

@kartben, what should I do to receive any feedback on this PR?

I would recommend trying to raise awareness in the #pr-help channel on Discord. Unfortunately there's no dedicated maintainer for this particular area / driver so that's likely your best bet. Thanks!

Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments
Also, you should add a note in doc/releases/migration-guide-4.1.rst about the changes you've been making to the API, i.e. indicate that all driver functions now take a device pointer since support for multiple instances has been added AND indicate that there is now also a new user_data parameter added to ft8xx_register_int and to the callback signature ft8xx_int_callback

{
(void)dev;
(void)user_data;

process_touch = true;
}

int main(void)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to your changes but this code should first perform an if device_is_ready(...) check

*/
typedef void (*ft8xx_int_callback)(void);
typedef void (*ft8xx_int_callback)(const struct device *dev, void *user_data);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding support for user data seems to be unrelated to adding support for multiple instances. It would be better to split the changes in different commits

@hubertmis hubertmis force-pushed the pr/ft8xx_multi_instance branch from 0e794d0 to a1467bd Compare January 26, 2025 19:53
@zephyrbot zephyrbot added the Release Notes To be mentioned in the release notes label Jan 26, 2025
@hubertmis
Copy link
Member Author

some comments Also, you should add a note in doc/releases/migration-guide-4.1.rst about the changes you've been making to the API, i.e. indicate that all driver functions now take a device pointer since support for multiple instances has been added AND indicate that there is now also a new user_data parameter added to ft8xx_register_int and to the callback signature ft8xx_int_callback

I've split the pull request into two commits and added notes to the migration guide.

Improve the FT8xx display driver to support multiple driver instances.

The ft8xx_reference_api.h still follows the FT8xx programming guides,
limiting the usage of the driver to one instance.

Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
The user of the ft8xx driver can register a callback function called
on a touch detection interrupt event. This function is extended to
include a user-defined pointer. The user-defined pointer helps to share
a callback with multiple driver instances.

Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
@hubertmis hubertmis force-pushed the pr/ft8xx_multi_instance branch from a1467bd to ca56e61 Compare January 26, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Display area: Samples Samples Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants