-
Notifications
You must be signed in to change notification settings - Fork 103
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
Refactor IAP Transport / Fix EASessions left open #1910
Refactor IAP Transport / Fix EASessions left open #1910
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly made some style formatting suggestions. Namely, private methods in the implementation files are prefixed by "sdl_". I tried to find most of them but I may have missed a few.
I did experience a crash on disconnect that should be a pretty easy fix.
I haven't had the chance yet to thoroughly test secondary transports and switching between BT and USB transports but testing the multisession and control protocols worked well.
Thanks Nicole,
I have been without a work computer all week. Going into Dearborn today to get it fixed or replaced. I will look at what you have here in detail by Monday if not by EOD today.
Mike
|
…or formatting updates per PR comments
Just commited changes based on PR comments. These include the following. |
There was a problem hiding this 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!
… iap session code
Codecov Report
@@ Coverage Diff @@
## develop #1910 +/- ##
===========================================
+ Coverage 84.66% 85.20% +0.54%
===========================================
Files 418 426 +8
Lines 20939 21376 +437
===========================================
+ Hits 17728 18214 +486
+ Misses 3211 3162 -49 |
I reviewed comments and made multiple updates for style requirements and also removed an used function. Hopefully I have everything this time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a few minor things to fix. A couple of comments from my previous reveiw were missed on the SDLIAPDataSession.m file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a few minor spacing issues - otherwise testing went well
Fixes #1799 (when used in conjunction with kmicha19-ford#2)
Fixes #1809
Fixes #1892
Fixes #1893
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
Unit Tests
Unit tests were updated for the refactored classes.
Core Tests
Summary
Changelog
Breaking Changes
Enhancements
Tasks Remaining:
CLA