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

WIP: Issue 749 app crashes when usb cord is pulled #752

Closed

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented Sep 21, 2017

Fixes #749
Fixes #679

This PR is not ready for review.

Risk

This PR makes no API changes.

Testing Plan

None written yet

Summary

  1. Notifications were not being unsubscribed properly in the SDLIAPTransport class, causing the app to crash when the USB cord was pulled.
  2. The lifecycle state machine was not taking into account that the SDL accessory could be disconnected after the app icon was setup but before the HMI was setup.
  3. The file manager state machine was not taking into account that the accessory could be disconnected before the list files request was returned.

Changelog

Tasks Remaining:

  • Add test cases for modified lifecycle state machine transitions
  • Add test cases for modified file manager state machine transitions

CLA

- Added protection statements to prevent calls to delegate if nil

Signed-off-by: NicoleYarroch <nicole@livio.io>
- fixed crash caused by strong reference cycle

Signed-off-by: NicoleYarroch <nicole@livio.io>
Signed-off-by: NicoleYarroch <nicole@livio.io>
Signed-off-by: NicoleYarroch <nicole@livio.io>
- life cycle state machine was not taking into account that the accessory could be disconnected after the app icon was setup but before the hmi was setup

Signed-off-by: NicoleYarroch <nicole@livio.io>
@NicoleYarroch NicoleYarroch self-assigned this Sep 21, 2017
@NicoleYarroch NicoleYarroch added the bug A defect in the library label Sep 21, 2017
@NicoleYarroch NicoleYarroch added this to the 5.0.0 milestone Sep 21, 2017
@codecov-io
Copy link

codecov-io commented Sep 21, 2017

Codecov Report

❗ No coverage uploaded for pull request base (release/5.0.0@f7fa476). Click here to learn what that means.
The diff coverage is 0%.

@@               Coverage Diff               @@
##             release/5.0.0    #752   +/-   ##
===============================================
  Coverage                 ?   54.3%           
===============================================
  Files                    ?     318           
  Lines                    ?    7316           
  Branches                 ?     668           
===============================================
  Hits                     ?    3973           
  Misses                   ?    3101           
  Partials                 ?     242

Signed-off-by: NicoleYarroch <nicole@livio.io>
…_pulled

Signed-off-by: NicoleYarroch <nicole@livio.io>

# Conflicts:
#	SmartDeviceLink/SDLLifecycleManager.m
Signed-off-by: NicoleYarroch <nicole@livio.io>
@joeljfischer
Copy link
Contributor

@NicoleYarroch The IAPTransport notifications were being unsubscribed correctly. You should not have to do each individually.

// We are sure to have a HMIStatus, set state to ready
[self.lifecycleStateMachine transitionToState:SDLLifecycleStateReady];
}

- (void)didEnterStateReady {
if ([self sdl_didAccessoryDisconnect]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we certain having these everywhere is necessary? We are getting a transport disconnect notification (transportDidDisconnect) if the accessory disconnected.

@joeljfischer
Copy link
Contributor

This was closed due to targeting a deleted branch. If this still needs to exist, re-open against master or develop

@NicoleYarroch NicoleYarroch deleted the bugfix/issue_749_app_crash_usb_cord_pulled branch February 7, 2018 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants