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

[spinel-cli] add vendor path option #106

Merged
merged 2 commits into from
Nov 24, 2020

Conversation

rlubos
Copy link
Member

@rlubos rlubos commented Nov 19, 2020

Introduce a --vendor-path option which allows to provide a custom location of the vendor package. It is also possible to provide this location with SPINEL_VENDOR_PATH environmental variable.

@rlubos
Copy link
Member Author

rlubos commented Nov 19, 2020

if options.vendor_path:
options.vendor_path = os.path.abspath(options.vendor_path)
vendor_path, vendor_module = os.path.split(options.vendor_path)
sys.path.insert(0, vendor_path)

Choose a reason for hiding this comment

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

I discussed this briefly with @rlubos today. I think this is more than warranted in this particular case, in order to solve this problem without extra complications and with very little risk. That said, I'd very much like @mbolivar-nordic to confirm this view.

Choose a reason for hiding this comment

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

That said, I'd very much like @mbolivar-nordic to confirm this view.

Yep, I think that's reasonable if you want the vendor module to be able to import other modules next to it.

@jwhui jwhui requested a review from LuDuda November 24, 2020 07:18
Copy link
Member

@LuDuda LuDuda left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

Copy link

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Looks good to me, tested with @rlubos locally.

@jwhui jwhui linked an issue Nov 24, 2020 that may be closed by this pull request
Copy link
Member

@jwhui jwhui left a comment

Choose a reason for hiding this comment

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

Thanks for this enhancement! 👍

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
…d#106)

There's no need to join VendorSpinelPropertyHandler class with
SpinelPropertyHandler since the methods in SPINEL_PROP_DISPATCH are
located in different objects anyway. This allows to move vendor
properties initialization into WpanApi class initializator, therefore
allowing to use custom Vendor module based on the command line argument.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Introduce a --vendor-path option which allows to provide a custom
location of the vendor package. It is also possible to provide this
location with SPINEL_VENDOR_PATH environmental variable.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
@jwhui jwhui merged commit c39176a into openthread:master Nov 24, 2020
jwhui pushed a commit that referenced this pull request Nov 24, 2020
There's no need to join VendorSpinelPropertyHandler class with
SpinelPropertyHandler since the methods in SPINEL_PROP_DISPATCH are
located in different objects anyway. This allows to move vendor
properties initialization into WpanApi class initializator, therefore
allowing to use custom Vendor module based on the command line argument.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option to specify vendor module location
5 participants