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

[microTVM] Zephyr: implement 'west_cmd' server option #8941

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

gromero
Copy link
Contributor

@gromero gromero commented Sep 6, 2021

Currently Zephyr Project API server lists option 'west_cmd' as an
option available in Zephyr platform by advertising it in PROJECT_OPTIONS
but that option is not used by any API method.

That commit adds that option to the server as a non-required option to
the flash() interface method, allowing the user to specify an
alternative path to the west tool. If that option is not specified the
Zephyr build system takes care of searching for west as a module (so
relying on West being available on Python, i.e. relying on
'python3 -m west').

Signed-off-by: Gustavo Romero gustavo.romero@linaro.org

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @gromero for fixing this! i have one suggestion here, but it would be good to bring this back and sorry for breaking it!

@@ -451,6 +451,11 @@ def flash(self, options):
recover_args.extend(_get_nrf_device_args(options))
check_call(recover_args, cwd=API_SERVER_DIR / "build")

if "west_cmd" in options:
west_cmd = options["west_cmd"]
cmake_args = ["cmake", "..", f"-DWEST={west_cmd}"]
Copy link
Contributor

Choose a reason for hiding this comment

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

i wonder if we should add this option to the original cmake invocation on line 395? i think it makes sense to have west_cmd, but thinking it's better to place there just in case this triggers a rebuild.

Copy link
Contributor Author

@gromero gromero Sep 9, 2021

Choose a reason for hiding this comment

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

@areusch Yeah, I was following what we had prior to Project API. Makes sense. Done in 395bcbe

Thanks for the review!

Currently Zephyr Project API server lists option 'west_cmd' as an
option available in Zephyr platform by advertising it in PROJECT_OPTIONS
but that option is not used by any API method.

That commit adds that option to the server as a non-required option to
the build() interface method, allowing the user to specify an
alternative path to the west tool. If that option is not specified the
Zephyr build system takes care of searching for west as a module (so
relying on West being available on Python, i.e. relying on
'python3 -m west').

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
@gromero gromero force-pushed the implement-zephyr-west-cmd-option branch from 4bd1c0d to 395bcbe Compare September 9, 2021 01:24
Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

LGTM

@leandron leandron merged commit b5c4aa3 into apache:main Sep 10, 2021
@leandron
Copy link
Contributor

This is merged now, thanks @gromero and @areusch

ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
Currently Zephyr Project API server lists option 'west_cmd' as an
option available in Zephyr platform by advertising it in PROJECT_OPTIONS
but that option is not used by any API method.

That commit adds that option to the server as a non-required option to
the build() interface method, allowing the user to specify an
alternative path to the west tool. If that option is not specified the
Zephyr build system takes care of searching for west as a module (so
relying on West being available on Python, i.e. relying on
'python3 -m west').

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
Currently Zephyr Project API server lists option 'west_cmd' as an
option available in Zephyr platform by advertising it in PROJECT_OPTIONS
but that option is not used by any API method.

That commit adds that option to the server as a non-required option to
the build() interface method, allowing the user to specify an
alternative path to the west tool. If that option is not specified the
Zephyr build system takes care of searching for west as a module (so
relying on West being available on Python, i.e. relying on
'python3 -m west').

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
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