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.

Sign up for GitHub

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

Jump to bottom

Service registration (#3545) part 3 - systems load data providers #3738

Merged
merged 30 commits into from
Apr 1, 2019
Merged

Service registration (#3545) part 3 - systems load data providers #3738

merged 30 commits into from
Apr 1, 2019

Conversation

david-c-kline
Copy link

This PR is a continuation of the work on #3545.

Changes include:

  • Add IMixedRealityServiceConfiguration
  • Add MixedRealityInputDataProviderConfiguration and MixedRealitySpatialObserverConfiguration
  • Rename BaseDeviceManager to BaseInputDeviceManager and update constructor args
  • Update the input system profile to contain the list of input device managers
  • Update the input system implementation to initiate load/unload of data providers
  • Add a spatial awareness profile to contain the collection of observers
  • Update the spatial awareness system to initiate load/unload of observers
  • Fix bugs in Enable for Speech (not checking for existing recognizer) and Dictation (possible re-registering of event handlers)
  • Add hand joint service and input simulator to input data providers
  • IMixedRealityDataProvider no longer inherits from IMixedRealityExtension service as they require a service (and are now registered by their parent service)
  • Limit registered services profile to only listing extension services
  • Rename "Additional Service Provider" to "Extension Services" in the Toolkit config inspector
  • Remove all data providers from the registered services profile

David Kline (ANALOG) added 27 commits March 21, 2019 14:57
move args from the paramter list to a separate array
b39a716
update input device managers
d5698d9
pass input profile to input data providers
0bee0eb
update to latest dev branch
a040ada
start adding support for specifying data providers
fc12c84
better guilayout scope handling
5ceb9a3
data provider support work
576b0a1
input data providers added to profile
256bada
add provider foldouts
8ac6fb0
re-introduce spatial awareness profile
4c45c93
add base observer profile class
6261875
add observers to spatial profile, inspector cleanup
94c14b5
fix editor error when updating the observer type
6586a2b
spatial system inspector ui fit/finish
33d606b
add mesh observer
f67a83f
loading input hardware managers via input system
ed8f2ee
register observers from spatial awareness system
90fa9ed
fix spatial profile passing
b6f1aea
fix spatial disable, avoid duplicate enable in speech
f8c956e
update speech fix, prevent dictation leak
1b19ea5
decouple data providers and extension services, cleanup
115195f
Merge remote-tracking branch 'upstream/mrtk_development' into service…
ab1432b 
…registrar.3.1
add playspace to constructor of input device managers
3a88987
merge mrtk_development_hl2 and fix conflicts
e61a0cf
cleanup incorrect interfaces
aa1a39d
update args list for base input device manager
8d67c96
add device mgr profile to input data provider config
065ca6d
@david-c-kline david-c-kline self-assigned this Mar 29, 2019
@luis-valverde-ms luis-valverde-ms self-requested a review April 1, 2019 11:49
wiwei
wiwei reviewed Apr 1, 2019
View reviewed changes
Assets/MixedRealityToolkit/Definitions/MixedRealityInputDataProviderConfiguration.cs Outdated
/// <param name="componentName"></param>
/// <param name="priority"></param>
/// <param name="runtimePlatform"></param>
/// <param name="profile"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe delete or or fill out some basic summary.

wiwei
wiwei reviewed Apr 1, 2019
View reviewed changes
Assets/MixedRealityToolkit.Providers/WindowsMixedReality/WindowsMixedRealityDeviceManager.cs
@@ -607,7 +625,8 @@ private void GestureRecognizer_HoldCompleted(HoldCompletedEventArgs args)
var controller = GetController(args.source, false);
if (controller != null)
{
MixedRealityToolkit.InputSystem.RaiseGestureCompleted(controller, holdAction);
IMixedRealityInputSystem inputSystem = Service as IMixedRealityInputSystem;
inputSystem.RaiseGestureCompleted(controller, holdAction);
Copy link
Contributor

Choose a reason for hiding this comment

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

inputSystem [](start = 16, length = 11)

Unrelated to this change, but do you know why there's only a ? check on the first function (i.e. GestureRecognizer_HoldStarted but none here or below - i.e. I would assume that GestureRecognizer_ManipulationStarted would need something similar if the issue is only on starts)

Copy link
Author

Choose a reason for hiding this comment

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

good question. i'll be sure to do a thorough scrub in the near future to ensure we are doing the right thing

wiwei
wiwei approved these changes Apr 1, 2019
View reviewed changes
Copy link
Contributor

@wiwei wiwei left a comment

Choose a reason for hiding this comment

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

:shipit:

@david-c-kline david-c-kline merged commit 801cae3 into microsoft:mrtk_development_hl2 Apr 1, 2019
@david-c-kline david-c-kline deleted the registrar.3.3 branch April 1, 2019 19:56
@StephenHodgson
Copy link
Contributor

Another rubber stamped PR without review from the community?

You guys aren't giving us enough time to review these.

@chbecker-ms
Copy link

This is a change we had baking for a while which was dependent on the mrtk_development_hl2 code which only moved up last week.

We are driving for transparency by doing as much work as we can in GitHub directly. This is one that didn't make the initial train that populated _hl2.

@wiwei wiwei mentioned this pull request Jun 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephenHodgson StephenHodgson StephenHodgson left review comments

@luis-valverde-ms luis-valverde-ms luis-valverde-ms approved these changes

@wiwei wiwei wiwei approved these changes

@radicalad radicalad Awaiting requested review from radicalad

@keveleigh keveleigh Awaiting requested review from keveleigh

@andreiborodin andreiborodin Awaiting requested review from andreiborodin

@maluoi maluoi Awaiting requested review from maluoi

@Railboy Railboy Awaiting requested review from Railboy

@chrisfromwork chrisfromwork Awaiting requested review from chrisfromwork

Assignees

@david-c-kline david-c-kline

Labels
None yet
Projects
None yet
Milestone
Mixed Reality Toolkit v2 Release Cand...
Development

Successfully merging this pull request may close these issues.
5 participants
@david-c-kline @StephenHodgson @chbecker-ms @wiwei @luis-valverde-ms