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

#3211 [SDL 0234] Proxy Library RPC Generation #3213

Merged
merged 26 commits into from
Feb 6, 2020
Merged

#3211 [SDL 0234] Proxy Library RPC Generation #3213

merged 26 commits into from
Feb 6, 2020

Conversation

vladmu
Copy link
Contributor

@vladmu vladmu commented Jan 16, 2020

Fixes #3211
This PR is [ready] for review.

Risk

This PR makes [no] API changes.

Testing Plan

Covered within existing unit tests and ATF tests.

Summary

This Pull Request provides changes of InterfaceGenerator according to the SDL-0234 proposal's requirements.

Changelog

Enhancements
  • Split existing InterfaceGenerator into InterfaceParser and InterfaceGenerator
  • Moved InterfaceParser into smartdevicelink/rpc_spec
  • Rejoined InterfaceParser as tools/rpc_spec Git submodule from smartdevicelink/rpc_spec
  • Refactored InterfaceGenerator to use Parser from tools/rpc_spec Git submodule and be compatible with Python 3.5

CLA

  * Split existing `InterfaceGenerator` into `InterfaceParser` and `InterfaceGenerator`
  * Moved InterfaceParser into `smartdevicelink/rpc_spec`
  * Rejoined `InterfaceParser` as `tools/rpc_spec` Git submodule from `smartdevicelink/rpc_spec`
  * Refactored `InterfaceGenerator` to use Parser from `tools/rpc_spec` Git submodule and be compatible with Python 3.5
@Jack-Byrne
Copy link
Collaborator

Jack-Byrne commented Jan 23, 2020

@vladmu Can the python install requirements.txt command be added to cores build script?

@ghost
Copy link

ghost commented Jan 27, 2020

@JackLivio moved the python install requirements.txt to core build script as per your comment.

@@ -12,7 +12,17 @@
import string
import uuid

from generator import Model
from model.array import Array
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to wrap all of the imports in a try/catch? If an error is encountered a log is printed that reminds the developer to run

git submodule init
git submodule update

Copy link
Contributor

Choose a reason for hiding this comment

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

this code tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
called from tools/InterfaceGenerator/Generator.py, where we adding appropriate paths:

ROOT = Path(__file__).absolute().parents[1]
sys.path.append(ROOT.joinpath('rpc_spec/InterfaceParser').as_posix())

and after this imports wrapped into try/catch:

try:
    ...
except ModuleNotFoundError as error:

so if issue with submodule appear it will be cached in Generator.py, so there is no options that such issue appear in SmartFactoryBase.py

Copy link
Collaborator

@Jack-Byrne Jack-Byrne Jan 29, 2020

Choose a reason for hiding this comment

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

Oh very good, thank you for the explanation.

Would it be possible to update the error message with some extra information?

Please make sure the RPC Spec Generator Submodule is initialized.
Check the sdl_core source directory in `tools/rpc_spec`.
Try running in the source directory:
$ git submodule init
$ git submodule update

Copy link
Contributor

Choose a reason for hiding this comment

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

extra information added

.gitmodules Outdated
@@ -0,0 +1,4 @@
[submodule "tools/rpc_spec"]
path = tools/rpc_spec
url = https://github.com/vladmu/rpc_spec.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

reminder to update this when merging

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vladmu @o-mishch I have merged the RPC_SPEC parser pr to the develop branch.

Please update the repository url to the smartdevicelink repo, and change the branch to develop. I will merge this PR after those changes.

Copy link
Contributor

@o-mishch o-mishch Feb 6, 2020

Choose a reason for hiding this comment

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

changed to:

[submodule "tools/rpc_spec"]
	path = tools/rpc_spec
	url = https://github.com/smartdevicelink/rpc_spec.git
	branch = develop

src/components/interfaces/CMakeLists.txt Show resolved Hide resolved
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.

3 participants