Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Integration for haptic manager into the SMM #753

Merged
merged 5 commits into from
Oct 2, 2017

Conversation

joeljfischer
Copy link
Contributor

@joeljfischer joeljfischer commented Sep 28, 2017

Fixes #698, #728

This PR is ready for review.

Risk

This PR makes major API changes.

Testing Plan

Tests must still be developed

Summary

This PR integrates the haptic manager (PR #751) into the streaming media manager (SMM) and the associated touch manager (TM). The flow now looks like this:

  1. Developer sets UIWindow on SDLStreamingMediaConfiguration, or not.
  2. If set, an SDLHapticManager is created on the SDLStreamingMediaManager. It is started with sending haptic RPCs disabled.
  3. If the video capability comes back with haptics enabled, sending haptic RPCs will be enabled on the haptic manager.
  4. The streaming media manager creates an SDLTouchManager with the haptic manager if it exists (depending on whether the window was set on the configuration).
  5. When the haptic manager is updated, if it is enabled, it will send a haptic RPC.
  6. If a touch comes through and the haptic manager exists, it will request a view for the touch location from the haptic manager and return it if it exists.

Changelog

Breaking Changes
  • SDLTouchManagerDelegate now has view passthrough on many delegate methods
Enhancements
  • Interface tweaks to SDLHapticHitTester, now just takes a point instead of a touch
  • SMM configuration has a window property that, if set, will create the haptic manager in the SMM
  • When the SMM receives video capabilities, it will check the hapticSpatialDataSupported property and enable / disable the haptic manager’s sending of RPCs based on it
  • TM now takes an optional haptic hit tester dependency to check view locations

Tasks Remaining:

  • Unit tests

CLA

* Interface tweaks to SDLHapticHitTester, now just takes a point instead of a touch
* SMM configuration has a window property that, if set, will create the haptic manager in the SMM
* When the SMM receives video capabilities, it will check the `hapticSpatialDataSupported` property and enable / disable the haptic manager’s sending of RPCs based on it
* TouchManager now takes an optional haptic hit tester to check view locations
* TouchManagerDelegate now has view passthrough on many delegate methods
@joeljfischer joeljfischer added this to the 5.0.0 milestone Sep 28, 2017
@joeljfischer joeljfischer self-assigned this Sep 28, 2017
@joeljfischer
Copy link
Contributor Author

Touch Manager tests were attempted to be updated, but OCMock crashed. They should be entirely rewritten without OCMock if possible.

if ((currentView.canBecomeFocused || isButton) && focusableSubviews.count == 0) {
//if current view is focusable and it doesn't have any focusable sub views then add the cuurent view and return
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling

if (configuration.window != nil) {
_hapticInterface = [[SDLHapticManager alloc] initWithWindow:configuration.window connectionManager:_connectionManager];
}
// __weak typeof(_hapticInterface) weakHaptic = _hapticInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out code can be removed

}
// __weak typeof(_hapticInterface) weakHaptic = _hapticInterface;
_touchManager = [[SDLTouchManager alloc] initWithHitTester:_hapticInterface];

Copy link
Contributor

Choose a reason for hiding this comment

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

I am getting a warning for _touchManager initialization.

Sending 'id _Nullable __strong' to parameter of incompatible type 'id _Nullable'

Casting _hapticInterface to id clears this warning for me.
_touchManager = [[SDLTouchManager alloc] initWithHitTester:(id)_hapticInterface]

@joeljfischer joeljfischer merged commit 70b9297 into release/5.0.0 Oct 2, 2017
@joeljfischer joeljfischer deleted the feature/haptic_integration branch October 2, 2017 19:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants