Emit event when oculus or vive controllers are connected #2505

fernandojsg · 2017-03-20T14:01:23Z

I didn't include any extra information as it can be accessed directly by the target

dmarcos · 2017-03-20T17:23:26Z

src/components/oculus-touch-controls.js

@@ -150,6 +150,7 @@ module.exports.Component = registerComponent('oculus-touch-controls', {
      controller: 0,
      rotationOffset: data.rotationOffset !== -999 ? data.rotationOffset : isRightHand ? -90 : 90
    });
+    el.emit('controller-connected', {type: 'oculus-touch-controller'});


no dashes controllerconnected is the convention

let's move these events to tracked-controls

won't work as there is no context provided to distinguish which component injected the tracked-controls. if that is provided to tracked-controls when injected, then I agree e.g. by expanding schema with something like source that is name of injecting component oculus-touch-controls, vive-controls, daydream-controller (which maybe should have been -controls), gearvr-controls and then providing source in the controllerconnected data

if we were to expand the tracked-controls schema, would the data be available when componentinitialized is fired for tracked-controls ? or is it only available in subsequent update ?

we can add the el reference to the details of the event

the case @fernandojsg has is same element with both oculus and vive components, need to know which injected; really the component reference, not the element

this is why I asked about moving the event to tracked-controls. tracked-controls will pass the Gamepad ID and el as details of the event. @fernandojsg would that work for you?

I think the idea was to avoid the ambiguity of having to know correspondence from gamepad.id to controller component. Another approach is to listen for componentinitialized or updated of tracked-controls, and look at .controllerPresent on the controller components -- but then not sure how to handle emulation (separate discussion)

machenmusik · 2017-03-20T17:47:20Z

(1) shouldn't this actually be done in the tracked-controls system that finds new controllers?
(2) shouldn't the type be the gamepad ID, if not the gamepad state itself? I am not entirely understanding what this provides in addition to ongamepadconnected events, maybe it is simply the abstraction?

machenmusik · 2017-03-20T17:50:00Z

would also like to understand the intended behavior in light of #2423 and #2446 e.g. what should be done if anything for emulated situations?

donmccurdy · 2017-03-20T19:09:43Z

+1 for using gamepad ID rather than a new string. And if a new string is needed, we should be consistent on -controls or -controller here.

machenmusik · 2017-03-20T19:46:42Z

if the general idea is to have the components re-emit their own version of gamepadconnected when they are going to inject, don't we also want something for gamepaddisconnected, and make it independent of 3DoF vs. 6DoF for example daydream controller and upcoming gearvr controller? (maybe even gearvr touchpad to the extent folks continue to use that with Carmel?)

fernandojsg · 2017-03-20T20:22:27Z

I needed this feature to detect when a gamepad is connected and determine (without need to redo most of the work done already by the core components) if it's an oculus or a vive controller.
In my particular case I needed to rotate the geometry of the gun to fit the controllers orientation differences. So I needed an event as the model could be loaded before the gamepad was already loaded.
I used -controls and -controllers to match the current components vive-controls and oculus-touch-controllers. I agree that we should define a consistent name for both, so probably we should start changin the name of these components? And even using singular instead of plural in their names?
I decided to send just the minimum amount of info because I could find the rest of info by looking at the target's componet, but f you feel that the ID could be useful too for sure we could include.
I'm open to any other proposal that could works for the same purposes.

machenmusik · 2017-03-20T20:32:30Z

@fernandojsg I see, so by type you really meant source component i.e. oculus-touch-controls or vive-controls which is not immediately obvious from the injected tracked-controls-- if we go back to the original decision to have all the different types of possible components attached, I think the pattern you are looking for is actually the auto-detect-controllers approach and then the particular injected component would be available through the standard eventing for new component addition, rather than only ambiguous tracked-controls. Given the current decision there, it seems as though the injected tracked-controls should have some indication of source component, so you can tell that the parent of the injected tracked-controls is vive or oculus. Does that sound like what you are looking for?

machenmusik · 2017-03-20T21:02:59Z

standard eventing for new component addition

Looks like there may be stale documentation
https://aframe.io/docs/0.5.0/core/entity.html#events_componentinit
https://aframe.io/docs/0.5.0/core/entity.html#events2_componentinitialized

If the name & id of the source/parent component doing the injection were available as part of as well as injected name/id/data, then this pattern would be generalizable to listening for and filtering componentinitialized which would be a nice solution. Thoughts?

machenmusik · 2017-03-22T05:57:42Z

Maybe no changes are required?
Have a look at which-controller-present in this CodePen...
http://codepen.io/machenmusik/pen/qroKWN

dmarcos · 2017-03-22T18:38:33Z

@fernandojsg would the example that @machenmusik points out in the codepen work for you?

fernandojsg · 2017-03-23T10:04:36Z

the @machenmusik proposal probably could works in most of the cases but I feel we're adding some extra steps that are not really necessary: adding a new component which-controller-present (should it be included in the core, or should we link from the aframe registry?), include that component in all the tracked-controls entities you want to check.
I feel it's the same if we'll create a is-model-loaded component and will emit a event when a json-model is loaded, and we should include it on every model we've loaded.
Maybe users could have also strange behaviour if this component is initialized after the tracked-controls is ready so it won't catch the events.
Probably a mix between the two approach could be to include that code inside the tracked-controls to remove my proposal code in each controls and emit these events directly from the tracked-controls.

fernandojsg · 2017-03-23T10:05:41Z

I've updated my PR with the component code of @machenmusik included directly on the tracked-controls . So we don't need that extra component and it's cleaner also as we don't need to remember to include that component. What do you think?

dmarcos · 2017-03-23T17:39:48Z

src/components/tracked-controls.js

+      const component = self.el.components[name];
+      if (component.controllerPresent) {
+        // Emit controllerpresent event.
+        self.el.emit('controllerpresent', {name: name, component: component});


The problem with this is that if the controller is not there when the component is initialized you won't be notified

I believe this is the case when you include directly tracked-controls instead of vive-controls or oculus-controls the specific controls inject tracked controls when the controllers is already available, but if you just inject tracked-controls by yourself you won't get 100% of the cases.

A controller could become available after the component / application is initialized

Yep, so what do to? Going back to the original approach to inject it when the controller is being injected? https://github.com/aframevr/aframe/blob/master/src/components/vive-controls.js#L94

vive-controls and oculus-touch-controls listen to gamepadconnected. I think everything should happen through tracked-controls. oculus-touch-controls and vive-controls should set tracked-controls on init and wait for an event when the controller becomes available.

we can then have trackedcontrollerconnected \ trackedcontrollerdisconnected. We can have a separate gamepad component?

well, tracked controller is really a controller with pose, so IMO we should stick with controllerconnected and controllerdisconnected (that would also fire for gamepads) -- oculus, vive, daydream can do their usual checking for ID prefix and pose (and maybe hand)... other (future) gamepad controls could attach to same event mechanism

to @fernandojsg 's original question though... which component should actually fire controllerpresent (which is when a higher-level component recognizes the actual presence of its controller, not just when a controller connected event is seen) -- the higher-level component after it injects (original PR), or by tracked-controls as per the latest here?

that's the goal of sceneEl.systems['tracked-controls'].subscribeToController(el, data, callback); you subscribe to the particular controller you're looking for so you don't have to manually check from a generic event

so the a-blast use case is kinda like hand-controls usage -- I think it's called shoot-controls -- but it needs to specifically know which higher-level controller component (oculus-touch-controls and vive-controls) to adjust model pose, etc.

I don't follow how oculus-touch-controls and vive-controls doing the subscription helps shoot-controls, and that subscription wouldn't help shoot-controls since it only knows oculus-touch-controls and vive-controls, it doesn't know the details of how they recognize what controller/gamepad.

machenmusik · 2017-04-04T03:27:58Z

I've updated my PR with the component code of @machenmusik included directly on the tracked-controls . So we don't need that extra component and it's cleaner also as we don't need to remember to include that component. What do you think?

So where did we end up with this one after all? Looking at it again, I think @fernandojsg proposal would work, and if other third-party components obey the controllerPresent convention, they could participate as well.

I believe this is the case when you include directly tracked-controls instead of vive-controls or oculus-controls the specific controls inject tracked controls when the controllers is already available, but if you just inject tracked-controls by yourself you won't get 100% of the cases.

True but if you inject tracked-controls yourself (rather than letting oculus-touch-controls, vive-controls, daydream-controls etc. do their detection) then you should know what you are doing and not need the signal event to tell you.

I imagine an entity wanting to subscribe to the controller but not use a tracked-controls component

For oculus-touch-controls, vive-controls, daydream-controls this is not possible as they rely upon tracked-controls for button / axis / pose update (and recording/replay takes full advantage of that fact).

dmarcos · 2017-04-20T18:53:39Z

Let's make tracked-controls properties id, prefixId and controller multi valuated to avoid collisions so the controller detection moves down from oculus-touch-controls, vive-controls to tracked-controls. We can then emit an event in updateGamepad when one of the controllers we're looking for is detected (https://github.com/aframevr/aframe/blob/master/src/components/tracked-controls.js#L77)

machenmusik · 2017-04-20T19:46:32Z

I'm not sure how that actually helps the original use case @fernandojsg was trying to solve (need to know whether oculus-touch-controls or vive-controls is actually connected) -- the upper level enjoys the abstraction of not having to know the raw gamepad id strings, but only the higher-level component abstractions,..

dmarcos · 2017-04-20T19:53:14Z

I was trying to solve duplicated code and emitting the event in one sweep. There's an easy way to emit the higher level event by passing the component to isControllPresent or mixing the function int the component (https://github.com/aframevr/aframe/blob/master/src/utils/tracked-controls.js#L34)

machenmusik · 2017-04-20T20:13:01Z

my recollection is that some components need to do their own filtering using getGamepadsByPrefix
... and also we only want the event to fire when things change, if we fire when isControllerPresent is true it will spam multiple times every tick.

machenmusik · 2017-04-20T20:18:03Z

thinking about problems separately,

to solve "how do i know which component injected this tracked-controls" just add a string to schema that higher level components fill in with their name when injecting. then you can read it even if you missed the event.

to fire an event when you see a new controller would ideally be gamepadconnected/disconnected from browser, but in practice i have not seen that work correctly so systems component would probably have to fire - perhaps on scene as we were discussing previously.

but putting the two together is slightly trickier as for fernando's use case, you only want the event AFTER you can read which component injected. So maybe fire event from tracked-controls schema update on that property.

fernandojsg · 2017-04-20T21:44:15Z

My main concern with moving the logic from vive/oculus-controls to tracked-controls is that it could grow up a lot as more controllers appears isn't it? Or maybe we could refactor after a while with something like registerController to specify the name and how it should detect it :)

So to follow with the PR with @dmarcos proposal, we should move checkIfControllerPresent and injectTrackedControls from the vive/oculus-controls to tracked-controls right? and the rest code should remain on each component.

fernandojsg · 2017-04-20T21:45:15Z

And so the trackedcontrollerconnected event will be emitted in the injectTrackedControls on tracked-controls similar as I did on the same function but on each component.

outdated

fernandojsg · 2017-04-21T11:13:52Z

I've refactored the code and moved all the inject and add/remove eventlistener to a new function in utils/tracked-controls checkControllerPresentAndSetup (Any suggestion for a better name maybe?) and it's working as expected.

The test are still not passing, as in checkControllerPresentAndSetup I'm calling isControllerPresent that we were mocked previously as they were included on the component itself, but now I'm calling it directly from the utils module and I can't think on an easy way to mock it so we could return true or false so it could inject correctly the listener and the component. Any idea?

machenmusik · 2017-04-21T15:56:30Z

@fernandojsg if you look at the vive-tracker PR, you can see a change to the way tests are mocked -- just change the system controllers list to what you need it to look like

fernandojsg · 2017-04-22T00:02:00Z

@machenmusik thanks! I've used your implementation and it's working as expected 👍

dmarcos · 2017-04-21T07:40:04Z

src/components/gearvr-controls.js

  },

  pause: function () {
    // Note that due to gamepadconnected event propagation issues, we don't rely on events.
    window.removeEventListener('gamepaddisconnected', this.checkIfControllerPresent, false);
-    this.removeControllersUpdateListener();


you want to keep these calls in the pause

dmarcos · 2017-04-22T00:03:22Z

examples/showcase/tracked-controls/index.html

@@ -57,5 +57,13 @@
         <a-entity light="type: ambient; color: #f4f4f4; intensity: 0.4;" position="-8 10 -18"></a-entity>
      </a-entity>
    </a-scene>
+    <script>


We can get rid of these logs

dmarcos · 2017-04-22T00:07:59Z

src/utils/tracked-controls.js

-    }
-    if (isPresent && queryObject.index) {
-      isPresent = index === queryObject.index; // need to use count of gamepads with idPrefix
+  if (gamepads) {


We can return early and save one level of indentation

dmarcos · 2017-04-22T00:16:08Z

Thank you! That's x2 bonus points for a PR that removes more code than adds.

fernandojsg · 2017-04-22T00:53:38Z

I've a question, Do we need to call addControllersUpdateListener on play even if the controller is not present?

machenmusik · 2017-04-22T05:19:00Z

i believe that due to issues with gsmepadconnected/disconnected propagation, that is still needed to reliably detect controllers, but would be happy to be proven wrong

fernandojsg · 2017-04-22T15:23:26Z

@machenmusik ok thanks, I've just keep them by now to keep the consistency between all the controllers components, we could address that later if needed.

@dmarcos fixed the issue with the remove/add listeners on play/pause

dmarcos · 2017-04-22T16:56:28Z

Thanks!

fernandojsg · 2017-04-22T18:51:16Z

@machenmusik I used false as I saw that we're initializing controllerPresent = false in all the init for each controller's component and that's why I used that value as default.

disable everGotGamepadEvent usage, seeing issues with latest Nightly make sure vive-controls doesn't match tracker bring in line with latest aframevr#2513 work around Nightly bug to get both controllers and trackers working at the same time bring vive-tracker in line with aframevr#2513 fix tests per separate discussion, use controllers list from system not getGamepadsByPrefix bring in line with aframevr#2505 make sure that queryObject values are used even if falsy e.g. index 0 tracked-controls needs to know about hand to filter properly and find vive trackers, given current Nightly bug. since empty string is always changed to default value (even when default is undefined), need to use 'none' to indicate empty-string hand re-clone tracker tests from controls use newly committed vive-tracker model add rotation property, which applies if no rotationOffset and allows full 3DOF rotation correct pose orientation for tracker to be what is generally expected of controllers fix default vive-tracker rotation correction, per discussion updateControllerModel for all tracked controls components; use fixed vive tracker model until published to CDN use new tracker model from CDN add rotation parameter apply rotationOffset the old way, to avoid precision issues add rotation offset test, with EPSILON change sentinel hand value to avoid node test failure zero pivot point update tracked-controls hand docs per discussion on PR fix duplicate play/pause presumably from botched merge

dmarcos previously requested changes Mar 20, 2017

View reviewed changes

fernandojsg force-pushed the add_connected branch from 066da71 to 225edad Compare March 23, 2017 10:04

fernandojsg force-pushed the add_connected branch 4 times, most recently from c14b13a to e71e9a6 Compare March 23, 2017 17:04

dmarcos reviewed Mar 23, 2017

View reviewed changes

ngokevin added this to the 0.6.0 milestone Mar 30, 2017

ngokevin added the needs review label Apr 6, 2017

dmarcos added needs rebase and removed needs review labels Apr 20, 2017

Move controllerPresent logic to utils.trackedcontrols

dcd3fd7

fernandojsg force-pushed the add_connected branch from c5aa5ce to dcd3fd7 Compare April 21, 2017 10:55

fernandojsg added needs review and removed needs rebase labels Apr 21, 2017

fernandojsg force-pushed the add_connected branch 2 times, most recently from 8663956 to a2bd1a6 Compare April 21, 2017 23:54

dmarcos reviewed Apr 22, 2017

View reviewed changes

fernandojsg force-pushed the add_connected branch 2 times, most recently from 452a359 to b4a7bac Compare April 22, 2017 00:10

fernandojsg force-pushed the add_connected branch from b4a7bac to 390efb5 Compare April 22, 2017 15:21

Fixed tests

f62a860

fernandojsg force-pushed the add_connected branch from 390efb5 to f62a860 Compare April 22, 2017 15:36

dmarcos merged commit bca9bdf into aframevr:master Apr 22, 2017

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 24, 2017

bring in line with aframevr#2505

70add4e

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 25, 2017

bring in line with aframevr#2505

6536c28

machenmusik added a commit to chenzlabs/aframe that referenced this pull request May 25, 2017

bring in line with aframevr#2505

418fcbd

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Jun 22, 2017

bring in line with aframevr#2505

be0401c

Emit event when oculus or vive controllers are connected #2505

Emit event when oculus or vive controllers are connected #2505

Conversation

fernandojsg commented Mar 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

machenmusik commented Mar 20, 2017

machenmusik commented Mar 20, 2017

donmccurdy commented Mar 20, 2017

machenmusik commented Mar 20, 2017

fernandojsg commented Mar 20, 2017

machenmusik commented Mar 20, 2017 • edited Loading

machenmusik commented Mar 20, 2017

machenmusik commented Mar 22, 2017

dmarcos commented Mar 22, 2017

fernandojsg commented Mar 23, 2017

fernandojsg commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik commented Apr 4, 2017 • edited Loading

dmarcos commented Apr 20, 2017

machenmusik commented Apr 20, 2017

dmarcos commented Apr 20, 2017 • edited Loading

machenmusik commented Apr 20, 2017

machenmusik commented Apr 20, 2017

fernandojsg commented Apr 20, 2017

fernandojsg commented Apr 20, 2017

fernandojsg commented Apr 21, 2017

machenmusik commented Apr 21, 2017

fernandojsg commented Apr 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmarcos commented Apr 22, 2017

fernandojsg commented Apr 22, 2017

machenmusik commented Apr 22, 2017

fernandojsg commented Apr 22, 2017

dmarcos commented Apr 22, 2017

fernandojsg commented Apr 22, 2017

machenmusik Mar 21, 2017 •

edited

Loading

machenmusik Mar 22, 2017 •

edited

Loading

machenmusik Mar 22, 2017 •

edited

Loading

machenmusik commented Mar 20, 2017 •

edited

Loading

machenmusik Mar 23, 2017 •

edited

Loading

machenmusik commented Apr 4, 2017 •

edited

Loading

dmarcos commented Apr 20, 2017 •

edited

Loading