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

Handle Assistant Button #21

Closed
wants to merge 2 commits into from
Closed

Conversation

nephros
Copy link

@nephros nephros commented Jan 25, 2023

This implements a small step to cover this feature request:

Xperia 10 iii: make Google assistant button a camera trigger

  1. by making the button recognized, and
  2. handling it the same as a "home" key

As mentioned in that thread, 2. is not most desireable solution, but at least now the button does something.

It would be much better if it were handled in a more interesting way. Therefore I mark this as a Draft, awaiting feedback.

Packages for the 4.4 SFOS release (mce v1.109.1) can be built by branching this OBS package.

nephros added 2 commits January 25, 2023 09:32
Prepares handling the as-yet-unused button on Xperia 10 III devices
@nephros nephros changed the title Assistant Handle Assistant Button Jan 25, 2023
@nephros
Copy link
Author

nephros commented Jan 25, 2023

Proposal:

If we can expose the event in Lipstick and/or on the bus for applications to handle, I'm sure the community will find and make their uses of it.
The OS does not necessarily need to define its function, or expose an UI in any way.

E.g. implementing a functionality similar to:

 --set-powerkey-dbus-action=<action_id,signal_argument|method_call_details>

for this key.

Other/more considerations can be found in this question to the community meeting.

@spiiroin
Copy link
Contributor

Couple of notes:

But but but... basically the policy with physical keys is:

  • power key is "owned" by mce - basically ui must ignore that it exists
  • all other keys are "owned" by ui - mce regulates when then inputs should be acted on by ui, but does not itself act on them / dictate how ui should handle them

And home key handling is sort of mixture in between those two - a quick hack driven by physical attributes of turing phone (fp sensor was on home key, using home key also for display wakeup allowed simpler unlock ux or stte). The code still works but is not used in any device / is candidate for removal -> there might be issues or at least it has not been given any thought for a while.

What I'm trying to say is: "works for now" mce config hack would be ok. But if there ever is need for using this key in apps / os wide in lipstick, it would be preferable to handle all of it at ui side. I'm not sure how one goes about doing such thing, but probably tweaking qt keymappings etc. @jpetrell @rainemak Any comments?

@nephros
Copy link
Author

nephros commented Jan 26, 2023

@spiiroin thanks for the comment. Good to know about the history of the home key.
It seems that the mixed handling of that makes evdev ignore it - so the .ini approach does not work as-is.

I have expanded a bit on the whole thing and its options in the question for the next IRC meeting.

https://forum.sailfishos.org/t/community-meeting-on-irc-2nd-february-2023/14091/10

If you prefer to have these discussions here, I can withdraw the question. Or wait for what comes out at the meeting, and continue this here after?

@pvuorela
Copy link

pvuorela commented Feb 1, 2023

I'm not either sure should this be mapped to the home key. Rather think we should handle that like the camera key. Either as separate comparison on Qt key level where there's camera related functionality, or then mapping the assistant key to the camera key.

For the current setup the lipstick-jolla has ShutterKeyHandler (main.qml) which internally reserves ResourcePolicy::SnapButtonResource and filters Qt::Key_Camera.

Then we have some event mapping in xkeyboard-config patches. If we could make it a camera button it could simplify the other code so could perhaps prefer that path. Maybe reserving the option to make this device specific mapping. Qtwayland was also involved in the event mappings to some extend. Recall there was a bit of mess with these.

@nephros
Copy link
Author

nephros commented Feb 9, 2023

I'm not sure I understand right, so bear with me for a bit:

I'm not either sure should this be mapped to the home key. Rather think we should handle that like the camera key.

Agreed, although my vision would be neither (also building on what @spiiroin points out):

  • home key: conceptually something that should cause an action by the main UI/Desktop, and no interaction by applications
  • camera key: very specific meaning, snap a picture (or maybe, in absence of an app which can do that, launch one that can)
  • assistant key: as intended by Google/Microsoft/Apple: launch a specific app or os-integrated function. Function in current SFOS: log a hex value. Opportunity: do something sexy.

I would like to have a hybrid : have the OS/Desktop/UI handle it principally, but enable users to choose from a (narrow) set of things that event should do.
Something like:

  • if nothing defined, e.g. act like the HOME (or lock, or whatever is a sensible default) function
  • if configured to do so, be a camera button
  • if configured to do so, launch a user-selected application

I think such an approach would offer much more appeal and consistency than just exposing the keycode to apps to do whatever (Because most apps won't handle it, and those that do won't do it in a consistent manner).


Now.

So something analogous the existent (mce->signal) home key handling in mce, but instead making a new event from it is NOT acceptable?
By this I mean what I do in https://github.com/nephros/mce/tree/assistant-contd, which is duplicating the home key code go get another event, the assistant key event.

Then we have some event mapping in xkeyboard-config patches. If we could make it a camera button it could simplify the other code so could perhaps prefer that path. Maybe reserving the option to make this device specific mapping. Qtwayland was also involved in the event mappings to some extend. Recall there was a bit of mess with these.

So the approach would be:

  • patch xkeyboard-config (like the changes to jolla_vndr/sbj)
  • possibly patch Qt (depending on maps from xkb)
  • possibly patch QtWayland
  • and then handle the key in lipstick, similar to a camera key (or some new way)?

I mean I can see the appeal in having the button event going through to Qt because then applications could do things with it. OTOH all this feels terribly hacky, lots of patches to carry to what amounts to abandoned code, with the end result of having a key event which most apps won't handle or care about.

@pvuorela
Copy link

I would like to have a hybrid : have the OS/Desktop/UI handle it principally, but enable users to choose from a (narrow) set of things that event should do.
Something like:

if nothing defined, e.g. act like the HOME (or lock, or whatever is a sensible default) function
if configured to do so, be a camera button

if configured to do so, launch a user-selected application

For the Xperia assistant key, if I didn't know it was used for that purpose, I would have just assumed it's a camera key. Probably many others too. Thus I think that's a good default.

The home key I'd think a bit different button, more like the android bottom row physical key or the such. If it would be exactly that, not sure should we be having configurability but rather just use it to take the user back to app switcher.

All in all configuration can be a bit hard, especially with a UI. Nice starting point is if the buttons do sensible things. And if there's configurability, should it be presented as configurability for the home, camera, or the assistant key? On Xperia the camera or assistant configurability could make some sense, but home key less so.

So something analogous the existent (mce->signal) home key handling in mce, but instead making a new event from it is NOT acceptable?

I'll leave it for Simo for what makes sense here, but handling that here feels to me it's hard to handle then otherwise, e.g. as camera key.

So the approach would be:
patch xkeyboard-config (like the changes to jolla_vndr/sbj)
possibly patch Qt (depending on maps from xkb)
possibly patch QtWayland
and then handle the key in lipstick, similar to a camera key (or some new way)?

Last no. If the event gets mapped to camera key, then all the lipstick and application handling you get for free. There it would look like just a camera button pressed.

The Qt side could be roughly the same no matter what the button is used for. I have a faint recollection that there was some evdev mapping where qt checked that the event is mapped to something and otherwise it's discarded.

So the real beef of the change should be just ~one line mapping in the xkeyboard-config. I'd try myself but don't have a assistant button equipped device available at the moment.

@pvuorela
Copy link

Can it act as a shutter button only in Camera(s) app? And elsewhere as user-specified (run .sh script), e.g. I'd want it to disable fingerprint service and lock the phone.

If it's some custom key event, we could handle that in the sailfish camera, but it's not feasible that random 3rd party apps start to follow all sorts of key events.

What could be done is mapping this to camera event and then do customization for that event so regardless whether the device has an assistant key or real camera key, both would get the same handling.

@nephros
Copy link
Author

nephros commented Feb 10, 2023

The Qt side could be roughly the same no matter what the button is used for. I have a faint recollection that there was some evdev mapping where qt checked that the event is mapped to something and otherwise it's discarded.

So the real beef of the change should be just ~one line mapping in the xkeyboard-config. I'd try myself but don't have a assistant button equipped device available at the moment.

Ok, is this detection of mapping compile-time or runtime?

Because me and others tried several runtime configs in this thread, including xkeyboard-config, udev/evdev, and others with no successful result.

@pvuorela
Copy link

Got a hold of a device and did it myself. It's a runtime thing.

mer-hybris/droid-hal-configs#258
sailfishos/xkeyboard-config#3

The hal configs need separate sync to xperia10 III repo, but can be manually tested by running kmap2qmap on the config and installing the new file to the device, then restarting lipstick.

@spiiroin
Copy link
Contributor

So something analogous the existent (mce->signal) home key handling in mce, but instead making a new event from it is NOT acceptable?

I'll leave it for Simo for what makes sense here, but handling that here feels to me it's hard to handle then otherwise, e.g. as camera key.

The basic problem is: once mce starts performing actions based some key, all other processes need to start ignoring that key or there will be problems -> the key becomes a special case that has to be dealt outside regular input handling stack -> there is costs vs benefit situation - and what those are can change/flip over time -> thought needs to be put in before implementing something that effectively comes fixed in stone part of os behavior and really hard to change later on.

  • assistant key: as intended by Google/Microsoft/Apple: launch a specific app or os-integrated function. Function in current SFOS: log a hex value. Opportunity: do something sexy.

+1 agreed ;-)

I would like to have a hybrid : have the OS/Desktop/UI handle it principally, but enable users to choose from a (narrow) set of things that event should do. Something like:

* if nothing defined, e.g. act like the HOME (or lock, or whatever is a sensible default) function

* if configured to do so, be a camera button

* if configured to do so, launch a user-selected application

Home is really special case because the behavior happens to involve display wakeup which then more or less requires that mce is brought in to the picture (and there is some desire to get rid of this special casing but - as said - it is hard). The rest of the above can be implemented without involving mce. And none of it can be implemented solely in mce. Which makes me not want to involve mce in this if possible. Naturally, it can be, but then it means there needs to be: explicit ownership of the physical key ala power key, policy for controlling which side of mce/ui fence owns the button ala volume keys, or carefully orchestrated rules to avoid mishaps ala escape key)

Also, I have a device that has camera button - not assistant button. But I'd still want to have this customizable thing. So I'd sort of vote for: map assistant key to camera key and then provide some configurable stuff at ui side for camera key handling. [Maybe with configurable side help from mce for handling display off/device locked situations if necesary]

@RikudouSage
Copy link

Just to add my opinion, making it a camera key is useless to me (and I presume others), it makes most sense to me as a flashlight button. Not making it configurable doesn't make sense to me at all, so whatever the implementation ends up being, please don't force your preferences on all of us and make it configurable in settings.

@Logic-gate
Copy link

If I may interject, and I know that this is not my area in any way shape or form. Why not opt for a simpler solution. A "service" based solution that does not require or rather depend on mce at all as @spiiroin pointed out.

Mapping event codes to specific actions can be done by other means whilst still adhering to the collective interest of having it configurable with relative ease. Is there an inherent advantage for having mce taking care of it as oppose to a script that just listens for an input and then runs a command?

@pvuorela
Copy link

I don't think this PR would be going forward. As Simo said, if something is to be done, we should map keys to camera button (which is done now), and then add configurability for how to handle the camera button. That change would go somewhere around lipstick-jolla / main.qml which now calls showViewfinder() on jolla-camera d-bus service.

@nephros
Copy link
Author

nephros commented Feb 20, 2023

Agreed. Closing, as the way forward has been adequately laid out, and this is not a discussion forum.

Thanks everyone for participating.

@nephros nephros closed this Feb 20, 2023
@Thaodan
Copy link
Contributor

Thaodan commented Feb 20, 2023

I don't think this PR would be going forward. As Simo said, if something is to be done, we should map keys to camera button (which is done now), and then add configurability for how to handle the camera button.

Some devices have a camera button and an assistant button mapping them into one would not make sense on such devices.

Examples are Xperia 1/5 III, Xperia 1/5 IV.

@RikudouSage
Copy link

It does not make sense at all, it's an assistant button, not a camera button. It should be registered as such and the user should be able to change what the assistant button does.

Also what @Thaodan said, has anyone actually given this any thought or are we just trying to rush it?

@pvuorela
Copy link

Some devices have a camera button and an assistant button mapping them into one would not make sense on such devices.

Examples are Xperia 1/5 III, Xperia 1/5 IV.

Shouldn't make a difference regarding this PR nor too much about other changes so far. But nevertheless a good point.

Support for such would need a bit more infrastructure around it. Something like:

  • device capability configuration and related qml api probably in nemo-qml-plugin-systemsettings / DeviceInfo & ssusysinfo.
  • xkeyboard-config mapping to different key between the assistant and camera key. And decision what to use, it likely needs to be a bit arbitrary choice, compare to current camera key mapping to XF86WebCam in lack of camera buttons on desktop computers, and focus to XF86ContrastAdjust (heh).
  • QtWayland mapping the arbitrary xkeyboard-config symbol to arbitrary Qt::Key_[...]. Not spotting immediately any obvious choice here either.
  • Some handling for the separate assistant key in lipstick-jolla.

PRs welcome, though these not being officially supported devices and me not having any of them might have some impact at least on me working on this stuff.

There could be also some better place for this discussion. Don't have better suggestions than the forum, though.

@nephros
Copy link
Author

nephros commented Feb 20, 2023

  • xkeyboard-config mapping to different key between the assistant and camera key. And decision what to use, it likely needs to be a bit arbitrary choice, compare to current camera key mapping to XF86WebCam in lack of camera buttons on desktop computers, and focus to XF86ContrastAdjust (heh).
  • QtWayland mapping the arbitrary xkeyboard-config symbol to arbitrary Qt::Key_[...]. Not spotting immediately any obvious choice here either.

One that seems fitting is XKB Help / XF86Help*) / Qt::Key_Help. (Upstream kmap2qmap should know about that too. Not too sure about the SFOS version).

(Tried that in some local tests but id didn't work for some reason. Ended up using XF86HomePage, which does.)

*) There are inconsistencies wrt naming of this. sometimes it's Help, sometimes XF86Help, sometimes both are defined.

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