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

Onboard: Add onboard virtual keyboard #67

Open
wants to merge 1 commit into
base: nilrt/master/next
Choose a base branch
from

Conversation

pratheekshasn
Copy link

@pratheekshasn pratheekshasn commented Sep 19, 2024

Summary of Changes

The PR adds the virtual keyboard "Onboard" to meta-openembedded.

Justification

#AB2491688 requires the keyboard to be present on scarthgap as well.

The virtual keyboard "Onboard" (present on kirkstone) had been removed from scarthgap due to build errors on python 3.12. This PR adds back the same with corrections required for python3.12.

image

Implementation

Changes that differ from kirkstone:

  1. The recipe inherits setuptools3 instead of distutils3. This change is required since distutils3 has been deprecated in python3.12 and the compile step fails. Since the compile fails, there are no packages that get installed.
  2. The recipe also installs the autostart file that gets generated at the staged folder's python site-packages folder to /etc/xdg/autostart folder. This is necessary because of the first change. setuptools3 installs files to the site-packages folder no matter what absolute path is specified as the prefix for the installation path.
  3. There is a new patch file that is now added to the recipe: onboard_hover_seg_fault.patch. This patch was found online here.
    Currently, when the mouse pointer hovers on a key, the app crashes with a segmentation fault, and this is what the patch fixes.

Other required changes can be seen in this PR in the meta-nilrt layer.

The keyboard can now be launched and used just like in kirkstone.

Testing

  • I have built the core package feed with this PR in place. (bitbake packagefeed-ni-core)

Tested by installing the built IPK on a VM.

  • Keyboard launch
  • Type on keyboard by clicking on the keys
  • Click on other windows (like the file explorer) to test that the keyboard goes to the background
  • Click on any window or textbox where something can be typed to test that the keyboard is back in focus and can type

Signed-off by: Pratheeksha S N [email protected]

@amstewart
Copy link

Have y'all discussed upstreaming this? If we cannot convince meta-OE upstream to revive the recipe, then we should instead move everything into meta-nilrt.

Copy link

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

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

Changes look fine.

But since we'd like to submit this upstream first, mentioning scarthgap in commit message is not ideal as we'd be submitting to master branch. So something like Onboard: Add onboard virtual keyboard would be better.

Once this ends up in upstream, we can do upstream merge to get the change into our repo.

Before sending upstream, please test it in poky and make sure the recipe is usable on its own i.e., it builds, produces valid package which is installable and usable without any NI specific changes from the meta-nilrt layer.

Copy link

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

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

@pratheekshasn pratheekshasn changed the title Onboard: Add onboard virtual keyboard to scarthgap Onboard: Add onboard virtual keyboard Oct 28, 2024

Signed-off by: Pratheeksha S N <[email protected]>

Upstream-Status: Not Submitted [This is being added to the meta-nilrt layer for now. If it is accepted upstream, it will be removed from here.]

Choose a reason for hiding this comment

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

This is not accurate, please fix.

Refer https://docs.yoctoproject.org/contributor-guide/recipe-style-guide.html#patch-upstream-status - I think it can be classified as "Inactive-Upstream", and the reason to [lastcommit: when (and/or) lastrelease: when].

I know we mention Not Submitted as one of the reasons in https://github.com/ni/meta-nilrt/blob/nilrt/master/next/docs/styleguide.md#L133 but haven't found mention of it upstream.

Copy link
Author

Choose a reason for hiding this comment

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

I have fixed this now.

Copy link

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

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

Looks good. Can be sent upstream now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants