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

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #272

Conversation

LitvinenkoIra
Copy link
Contributor

@LitvinenkoIra LitvinenkoIra commented Jul 31, 2020

Mobile API updates according to the #273

smartdevicelink/sdl_core#3465

This PR is ready for review.

IGapchuk and others added 3 commits July 23, 2020 11:36
Currently InterfaceParser is not able to generate recursive structures
i.e. one or several structure attributes have its own type e.g.:
  struct VideoStreamingCapability {
    ...
    VideoStreamingCapability additionalVideoStreamingCapabilities;
  }
because structure type fully defined only when all of it attributes are
defined.

With this commit InterfaceParser is able to handle such problem.
…bility_to_update_video_streaming_capabilities
@LitvinenkoIra LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch from 5f7c9ed to 7245fbd Compare August 7, 2020 10:42
@LitvinenkoIra
Copy link
Contributor Author

@atiwari9 please review this PR

Copy link

@atiwari9 atiwari9 left a comment

Choose a reason for hiding this comment

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

Approved the PR with below commits:

Test Results:

Logs:
2020-08-19_15-57-51.zip


TOTAL: 44
PASSED: 42
FAILED: 0
ABORTED: 0
SKIPPED: 2

SDL Core: 36f2f377929c52f84cb4b4216795fcc5c96f58b4
RPC Spec: 4d1312648b78b69f881d97b4b9fd0d8291736750
SDL HMI: 788499f2078e522d710dce3f413045277b278785
Test Scripts: e6d7f578d52c3b932bc2c5ed9465e9d0c8fd8979

Please resolve any merge conflicts.

@LitvinenkoIra LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch 2 times, most recently from e87238a to 85e868f Compare December 15, 2020 14:52
@LitvinenkoIra
Copy link
Contributor Author

@jordynmackool @JackLivio This PR is ready for Livio review. Thank you!

MOBILE_API.xml Outdated Show resolved Hide resolved
InterfaceParser/parsers/rpc_base.py Outdated Show resolved Hide resolved
@ShobhitAd
Copy link
Contributor

@LitvinenkoIra There seem to be some changes from develop in the MOBILE_API.xml file which are being removed/overwritten in this PR (such as themediaClock param being changed in https://github.com/smartdevicelink/rpc_spec/pull/272/files#diff-42ba46e1d01a876881e11902ad4a762c5040d0e7b089b17e7bfaaad9b10cd932R5833 and the countRate param being removed in https://github.com/smartdevicelink/rpc_spec/pull/272/files#diff-42ba46e1d01a876881e11902ad4a762c5040d0e7b089b17e7bfaaad9b10cd932L6040)

Is it possible to revert that commit and try merging in the changes from develop again (or using the develop version and adding in the new video streaming capability parameters again)?

LitvinenkoIra added 2 commits January 28, 2021 17:51
@LitvinenkoIra LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch from 8a91261 to 166c5be Compare January 28, 2021 15:55
@LitvinenkoIra
Copy link
Contributor Author

@ShobhitAd Oh I see, sorry for that. I've updated the merge, but I had to force push the branch. Now it should be ok

@LitvinenkoIra
Copy link
Contributor Author

LitvinenkoIra commented Jan 28, 2021

@ShobhitAd I can see that this PR is still have conflicts, but I'm not sure that the latest commit from the develop branch is correct, could you please check it 0110478
For example it seems like changes related to the distanceToManeuver 0110478#diff-42ba46e1d01a876881e11902ad4a762c5040d0e7b089b17e7bfaaad9b10cd932R7291 shouldn't be there since they are a part of #304

@ShobhitAd
Copy link
Contributor

@ShobhitAd I can see that this PR is still have conflicts, but I'm not sure that the latest commit from the develop branch is correct, could you please check it 0110478
For example it seems like changes related to the distanceToManeuver 0110478#diff-42ba46e1d01a876881e11902ad4a762c5040d0e7b089b17e7bfaaad9b10cd932R7291 shouldn't be there since they are a part of #304

@LitvinenkoIra Merged #313 to fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants