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

Added support for setting a custom CarWindow screen resolution #923

Merged
merged 8 commits into from
Apr 9, 2018

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented Apr 4, 2018

Fixes #908

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

Test cases added to the SDLStreamingMediaLifecycleManagerSpec.

Summary

  • Preferred resolution is now set even if GetSystemCapability is not supported by Core.
  • Preferred resolution is now set when the videoAckPayload does not return values for width and height.
  • The SDLStreamingMediaManagerDataSource file is now public.

Changelog

Breaking Changes

None

Tasks Remaining:

  • Write test cases

CLA

- Preferred resolution is now set, even if video capabilities is not supported by Core
- Preferred resolution is now set when the `videoAckPayload` does not return a width or height
- Exposed the `SDLStreamingMediaManagerDataSource` files
@NicoleYarroch NicoleYarroch changed the title WIP: Add support for setting a custom Car Window screen resolution WIP: Add support for setting a custom CarWindow screen resolution Apr 4, 2018
@joeljfischer joeljfischer self-requested a review April 4, 2018 17:51
@joeljfischer joeljfischer added bug A defect in the library enhancement labels Apr 4, 2018
@joeljfischer joeljfischer added this to the 5.3.0 milestone Apr 4, 2018
@NicoleYarroch NicoleYarroch changed the title WIP: Add support for setting a custom CarWindow screen resolution Added support for setting a custom CarWindow screen resolution Apr 5, 2018
@NicoleYarroch NicoleYarroch self-assigned this Apr 5, 2018
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

Just a few small changes

} // else we are using the screen size we got from the RAIR earlier
} else {
// If a preferred resolution was set, use the first option to set the screen size
if (self.preferredResolutions.count > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we combine the else above and this if?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

} else {
// If a preferred resolution was set, use the first option to set the screen size
if (self.preferredResolutions.count > 0) {
SDLImageResolution *preferredResolution = self.preferredResolutions[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

self.preferredResolutions.firstObject is a tiny change, but I personally like to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@joeljfischer joeljfischer merged commit 1a33c27 into develop Apr 9, 2018
@joeljfischer joeljfischer deleted the bugfix/issue_908_car_window_screen_resolution branch April 9, 2018 14:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug A defect in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants