-
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
Implement SDL-0224 Navigation Subscription Buttons #1342
Implement SDL-0224 Navigation Subscription Buttons #1342
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.
Left some comments
SmartDeviceLink/SDLButtonName.h
Outdated
/** | ||
* Represents a Navigate to center button. | ||
*/ | ||
extern SDLButtonName const SDLButtonNameNavCenter; |
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.
This should be SDLButtonNameNavCenterLocation
to match the name: NAV_CENTER_LOCATION
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.
done
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.
The SDLButtonNameNavCenter
name still needs to be updated.
SmartDeviceLink/SDLButtonName.h
Outdated
/** | ||
* Represents a Zoom in button. | ||
*/ | ||
extern SDLButtonName const SDLButtonNameZoomIn; |
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.
SDLButtonNameZoomIn
should be SDLButtonNameNavZoomIn
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.
All the nav buttons should have Nav
in the button name.
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.
done
SmartDeviceLink/SDLButtonName.h
Outdated
/** | ||
* Represents a Pan up button | ||
*/ | ||
extern SDLButtonName const SDLButtonNamePanUP; |
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.
Fix the case for UP
to Up
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.
fixed
SmartDeviceLink/SDLButtonName.h
Outdated
/* | ||
* Represents a Rotate counterclockwise button | ||
*/ | ||
extern SDLButtonName const SDLButtonNameRotateCounterClockWise; |
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.
Fix the case for ClockWise
to Clockwise
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.
done
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.
Left some comments
SmartDeviceLink/SDLButtonName.h
Outdated
/** | ||
* Represents a Navigate to center button. | ||
*/ | ||
extern SDLButtonName const SDLButtonNameNavCenter; |
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.
The SDLButtonNameNavCenter
name still needs to be updated.
SmartDeviceLink/SDLButtonName.h
Outdated
/** | ||
* Represents a Pan down/right button | ||
*/ | ||
extern SDLButtonName const SDLButtonNamePanDownRight; |
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.
The button name is missing the Nav
Fixes #1269
This PR is ready for review.
Risk
This PR makes minor API changes.
Summary
Adding enum values so navigation apps can subscribe to navigation hard buttons
CLA