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

OnProxyOpened is mistakenly called for all service types #358

Closed
joeljfischer opened this issue Feb 22, 2016 · 6 comments
Closed

OnProxyOpened is mistakenly called for all service types #358

joeljfischer opened this issue Feb 22, 2016 · 6 comments
Labels
bug A defect in the library
Milestone

Comments

@joeljfischer
Copy link
Contributor

Bug Report

Currently, it appears that we are calling onProxyOpened from SDLProxy L#183 for every service type. I am unsure if the || should be && or if there's a reason the line is as it is.

Reproduction Steps
  1. Connect to Core using RPC service
  2. Enable another service, such as the video service
Expected Behavior

SDLProxy does not call onProxyOpened on its delegate.

Observed Behavior

SDLProxy calls onProxyOpened on its delegate.

OS & Version Information
  • iOS Version: 9.2.1
  • SDL iOS Version: 4.0.1
  • Testing Against: SDL Core 4.0.4
@joeljfischer joeljfischer added the bug A defect in the library label Feb 22, 2016
@joeljfischer joeljfischer added this to the 4.0.X milestone Feb 22, 2016
@joeljfischer
Copy link
Contributor Author

@asm09fsu @Heath-FoMoCo @justinjdickow Do you know why we have a version check in the line linked above? Do you know if there's bad ramifications for removing || [SDLGlobals globals].protocolVersion >= 2?

@Heath-FoMoCo
Copy link

@khburdette might have to help. He might be the most familiar with how the different sessions are notified of connection . @mrapitis can you comment based on android?

@mrapitis
Copy link
Contributor

On android we only call onProxyOpened when a new (unencrypted) RPC session is established. I'm not sure why a protocol version check is occurring on iOS.

@joeljfischer
Copy link
Contributor Author

Okay, I'm going to remove it in the security branch since it's causing some oddities for video / audio. If we discover something in testing, we can deal with it then.

@Heath-FoMoCo
Copy link

@joeljfischer 👍

@joeljfischer joeljfischer modified the milestones: 4.0.3, 4.0.X, 4.0.4 Mar 3, 2016
@joeljfischer
Copy link
Contributor Author

Fixed in a4f1eac

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

No branches or pull requests

3 participants