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

Issue 799 easessions close on disconnect #1

Closed
wants to merge 8 commits into from

Conversation

kmicha19-ford
Copy link
Owner

@kmicha19-ford kmicha19-ford commented Jan 22, 2021

Fixes smartdevicelink#1799 (when used in conjunction with #2)
Fixes smartdevicelink#1809
Fixes smartdevicelink#1892
Fixes smartdevicelink#1893

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have tested this PR against Core and verified behavior.

Unit Tests

Unit tests were updated for the refactored classes.

Core Tests

  • Built the Obj-C example app and verified that all interactions work
    • iAP disconnect / reconnect
    • LockScreen on/off rapidly
  • Built the Swift example app and verified that all interactions work
    • iAP disconnect / reconnect
    • LockScreen on/off rapidly

Summary

  • Refactors iAPSession code that uses the Apple External Accessory Frameworks to create, open, use, and close EASessions.

Changelog

Breaking Changes
  • None
Enhancements
  • Moves all managment of EASession creation, open, and closing into a single class, SDLIAPSession
  • Eliminates the use of NSThread from SDLIAPDataSession
  • SDLIAPSession now uses a DispatchQueue to provide the required thread for EASession
  • Uses composition instead of inheritance, SDLIAPControlSession and SDLIAPDataSession now use SDLIAPSession instead of being subclasses of it
  • Provides a short simple code path for closing EASessions
  • Eliminates over two hundred lines of code

Tasks Remaining:

  • More unit tests

CLA

@kmicha19-ford kmicha19-ford changed the title Issue 799 easessions left open Issue 799 easessions close on disconnect Feb 1, 2021
@kmicha19-ford
Copy link
Owner Author

Will do this PR again today.

Copy link

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

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

I think a few things were missed from my last review. Can you tag me when you have a fix for the crash I noted in the last review? Thanks!

Sorry, left this review on the wrong PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants