toyota-connected / ivi-homescreen

Embedded Flutter runtime targeting Embedded Linux with Wayland
Other
247 stars 33 forks source link

agl shell protocol v2 #83

Closed jwinarske closed 2 years ago

jwinarske commented 2 years ago

-basic implementation

Signed-off-by: Joel Winarske joel.winarske@gmail.com

jwinarske commented 2 years ago

@mv0

mv0 commented 2 years ago

Personally, I would split these into 2 different patches. Easier to track if there's something wrong, and easier to review.

I'd also invest some time into having a commit description which says a few words about why we're updating to v3 would be beneficial, and if you're still going to keep these changes in one big lump, to also include some words about the fact this update will also include a bump to v2 implicitly when doing the update to version 3, and with it, what does that entail.

Note that I've only skimmed it, I'll be doing in a few days a more through review, but semantically looks it the right thing.

jwinarske commented 2 years ago

@mv0 I'm not entirely clear why v2 and v3 are needed. Perhaps you can paraphrase why v2 and v3?

Also I have tested on v1, but need to validate with v2 and v3. How do I get an AGL image with v2 and then v3?

mv0 commented 2 years ago

An update to support protocol v3 requires having support for v2, or at least provide some empty stubs for v2. Can't jump from a version to another without adding intermediary support. That's how it is.

The protocol v2 is not obligatory to be supported, but I've noticed that in some cases where we do not explicitly have a knob in the toolkit to tell whether the application is a regular one (which doesn't need to bind to the agl-shell protocol) or a one that needs to implement agl-shell protocol might result in terminating the wayland connection. That happens because we can't have multiple clients bind to the agl-shell protocol interface. This was particularly visible when I've tried using regular flutter applications with other shell clients (qt homescreen, or WAM/chromum), basically mashing together different kind of toolkits in the same image. These two events provide a race-free method in which the clients can tell if they're in charge (of being the shell client) or their just regular applications. Alternatively, like WAM/chromium has been doing for some time now, is to have an explicit knob at the command line which tells it beforehand what type of of application it's running: regular html5 application, or an shell client. Like I've said above, you do not need to explicitly implement if you have other means to specify which type of application it is running. Using the protocol can provide the same thing, programmatically, without fearing that the wayland connection might be severed, and implicitly taking down your application.

The protocol v3 update was mostly driven as fix for SPEC-4528. By default the compositor will display/activate applications as soon they're started, a feature which I've called activate-by-default, and which is turned on implicitly. Users can choose to disable that in such a way that activation is entirely driven the shell client. For instance from a certain application using some RPC method, you'd start another application, and at some point in time, you'd want to display it (activate it). Turns out that implicitly having this activation by default papered over an issue when don't have activation by default turned on. Supporting both use-cases (activation by default on and off) turned out to be cluster of problems and regression sover time and I want to have an explicit activation happening at start-up. Problem is we don't have a way to know when exactly the application has started and it is the right time to activate it. What I've discovered with SPEC-4528 is that at start-up we send the activation request way to soon for it to matter but we've never had any issue as activate by default was implicitly turned on. Having that turned off (and implicitly removed any code to have it turned on at all) requires to orchestrate correctly start-up of applications with activation. v3 protocol update provides an event for the shell client, to let it know when application started and can further issue the activation request. With this v3 we also add a few more events, I believe useful to have: termination/deactivation along side started/activated.

We don't bundle together multiple protocol updates in one go to keep things easier to review and be fairly sane. Protocol development is one the hardest things in wayland to get it right. Ideally, I should've had v2 by default at the beginning.

Also I have tested on v1, but need to validate with v2 and v3. How do I get an AGL image with v2 and then v3?

v2 is now in master of agl-compositor but v3 is still in review phase. Once I get v3 included in master, a SRCREV bump in the recipe would pick those changes up.

If you want to test those now, I'd normally use EXTERNALSRC to fetch up master for agl-compositor, cherry-pick or checkout v3 changes top and try that way.

I'm hoping that this week I can also get v3 also in master, so just doing a SRCREV bump in the recipe would be sufficient to test it.

It will probably take a week, or so, to add patch to officially bump SRCREV, which I'll add probably next week. The SRCREV bump also includes some bug-fixes which I want to get it as well, that's why I'm still waiting for doing that.

mv0 commented 2 years ago

Testing this out should be now much easier, if you pull in/use the https://gerrit.automotivelinux.org/gerrit/c/AGL/meta-agl/+/27989

mv0 commented 2 years ago

Ah, so you're just doing a v2 update for the time being. Looks good to me now.