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

Implement Revisions - SDL 0099 New remote control modules, SDL 0175 Updating DOP value range for GPS notification #1046

Conversation

mrapitis
Copy link
Contributor

@mrapitis mrapitis commented Aug 10, 2018

Fixes #1003, #755

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Added new unit tests and expanded existing ones

Summary

Updated RPC's, structs, enums as defined in Revise New remote control modules proposal:
smartdevicelink/sdl_evolution#572
&
Updating DOP value range for GPS notification proposal:
smartdevicelink/sdl_evolution#577

For simplicity of review, issues have been combined into single PR as changes are dependent on each other.

CLA

  • I have signed the CLA

@mrapitis mrapitis force-pushed the feature/SDL-0099-remote-control-light-audio-revise branch from a29fb4b to 86a63d6 Compare August 10, 2018 18:24
@joeljfischer joeljfischer added the proposal Accepted SDL Evolution Proposal label Aug 13, 2018
@joeljfischer
Copy link
Contributor

joeljfischer commented Aug 14, 2018

@mrapitis I'm seeing unit tests fail to compile due to an import of an old file (SDLSISDataSpec still uses GPSLocation).

@mrapitis
Copy link
Contributor Author

@joeljfischer we have removed remaining references to GPSLocation. Please re-review when time permits. Thanks!

…io-revise

# Conflicts:
#	SmartDeviceLink-iOS.xcodeproj/project.pbxproj
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.

This looks good. I just need to wait for the corresponding Core implementation to test against.

@mrapitis
Copy link
Contributor Author

@joeljfischer we have updated the PR to include latest proposal corrections for https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0099-new-remote-control-modules-and-parameters.md

From:

<element name="REAR_LEFT_BREAK_LIGHT" value="16"/>
<element name="REAR_RIGHT_BREAK_LIGHT" value="17"/>

To:

<element name="REAR_LEFT_BRAKE_LIGHT" value="16"/>
<element name="REAR_RIGHT_BRAKE_LIGHT" value="17"/>

Please re-review when time permits. Thanks!

@joeljfischer
Copy link
Contributor

joeljfischer commented Aug 27, 2018

@mrapitis I have a few additional notes:

  • SDLLightControlData initWithLightStateArray -> initWithLightStates
  • SDLEqualizerSettings initWithChannelId:channelName:channelSetting: should not exist, as channelName is readonly.
  • SDLRadioControlData initWithFrequencyInteger:frequencyFraction:band:hdChannel:radioEnable:sisData: should not initialize sisData because it is readonly, however, it should initialize hdRadioEnable which is writable but not included.

@mrapitis mrapitis force-pushed the feature/SDL-0099-remote-control-light-audio-revise branch from 07b26fd to ac9245d Compare August 28, 2018 13:36
@mrapitis
Copy link
Contributor Author

@joeljfischer PR has been updated with provided notes. Please re-review when time permits. Thanks!

@joeljfischer joeljfischer merged commit 4b343d3 into smartdevicelink:develop Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Accepted SDL Evolution Proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants