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

Virtual keyboard #448

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Virtual keyboard #448

wants to merge 13 commits into from

Conversation

constantegonzalez
Copy link
Contributor

Jaxson added a initalization function and was tested on vpsmalls manually deploying the folder and the change to adhoc_browser_pool.py
the actual deploy from setup.py change is not tested

@minshallj
Copy link
Contributor

what's the license from this original extension?

@jandro-air
Copy link
Contributor

From Tino in R+D zulip stream:

https://chrome.google.com/webstore/detail/virtual-keyboard/pflmllfnnabikmfkkaddkoolinlfninn
this is the page for the extension, not open source (or is it?, looked closer at the files, I think all the code is there)
I can't find a license but it repeats things like:
"Begin using this virtual keyboard now. It is 100% free..."
but also:
"Disclaimer: Please note this extension is NOT made by Google and is made by an independent development team. All copyrights belong to their respective owners."
the makers page:
https://apps.xontab.com/VirtualKeyboard/
their theme is free apps :shrub:

@jandro-air
Copy link
Contributor

Fwiw, this does seem to work when deploying manually. One thing to note - in addition to having a keyboard solution for Solos, we ideally would want to use this for Podium systems where we deploy the VUE Touchscreen.

Tino and I tried deploying this, but adhoc browser would crash when launching through the CMS and static_browser / touchscreen_browser didn't seem to accept the extensions ros param we tried to give it even with the virtual keyboard files copied over.

We might want to extend touchscreen browser to support the desired extension. @minshallj maybe you have some insight on this?

@jandro-air
Copy link
Contributor

Tested this on EPHQ using adhoc browser, seemed to work well enough. Would be great to implement support for this on the touchscreen_browser and / or static browser nodes (wherever is appropriate, idk how the other extensions would affect what we currently have implemented).

@jandro-air
Copy link
Contributor

touchscreen_browser node running virtual keyboard extension:
image

This wasn't too bad - did some digging into what needed extending, should be simple. We can copy the two extensions lines from static_browser.py over to touchscreen.py, and then managed_browser.py just needs an extensions = extensions.split() after line 128.

Noticed two things while testing:

  1. Looks like adding the change might cause some package in lg_sv to crash (will need more in-depth looking, seems like panovideo maybe?), but i assume this is a fairly easy logic change to fix
  2. We are setting the user_agent for static_browser.py and touchscreen.py... I think as @constantegonzalez has established in previous research (and an email chain about cbre-mclean security) that we should be safe to remove this, unless anyone has concerns about this.

@jandro-air
Copy link
Contributor

note, need to add the following to the touchscreen / static_browser if we want to enable the keyboard this way:

<param name="extensions" value="/opt/ros/melodic/lib/python2.7/dist-packages/lg_common/extensions/virtual_keyboard"/>

@jandro-air
Copy link
Contributor

Alright, with Tino's help, fixed the other errors. Now, two things left:

  1. the sizing of the keyboard on wider screens seems a bit too small.
  2. the "enter" button was not actually sending input. The keys do type, but "enter" doesn't actually trigger the action, which we'll need for searching and such.

@jandro-air
Copy link
Contributor

Tested a bunch with Jaxson to attempt to implement his changes. Had all kinds of issues with sizing and with input. Found out ultimately we could input fine on textarea components, but not input components, which is what would have at least mattered for the touchscreen if not certain sites.

Found a new keyboard extension that does appear to work as expected instead, and since we already did all the ROS stuff, works essentially out of the box. There are a couple issues - the "enter" button doesn't submit forms, and the keyboard pops up at the bottom of a page, underneath all other elements and scrolling the page up. This is a problem for the touchscreen interface, since it sort of breaks the experience for the user, even though the keyboard input aspect works. @Jaxson-Baerg will take a look to see if it's worth doing something with this.

We also need to test the keyboard at a larger size, like on a 4k display. @constantegonzalez will be looking into this.

Last thing - I was able to get everything running as expected on the podium / touchscreen with using the static_browser node, but not the touchscreen_browser node. Not sure why that is, and we likely just need to copy some stuff over to the touchscreen node. But, if we think it's not problematic to use the static_browser for typical podium systems / solos, then it should be fine to run. I did not find any differences with the launch command for either node at least, but not sure if there are other differences present between the two nodes. Anyways, might need some insight from @minshallj on that.

@Jaxson-Baerg
Copy link

Tested and dug into the out-of-box keyboard that Jandro found and couldn't see a feasible or time-friendly way to modify the logic in order to fix the "unable to submit forms" bug that Jandro found. I did find however that it would be relatively easy to solve the DOM issue where the keyboard "pushed up the page", but I won't invest any more hours into this until we solidify a plan of action to move forward. Jandro will take this to the team to plan a direction

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.

4 participants