-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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] Update Zephyr version and Zephyr SDK version #13818
Conversation
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
487e094
to
8c906a9
Compare
5ac369b
to
23f410d
Compare
@tvm-bot rerun |
23f410d
to
42ae754
Compare
apps/microtvm/arduino/template_project/launch_microtvm_api_server.sh
Outdated
Show resolved
Hide resolved
apps/microtvm/zephyr/template_project/launch_microtvm_api_server.sh
Outdated
Show resolved
Hide resolved
@@ -40,15 +42,36 @@ | |||
|
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.
Is there any test that shows how to pass the west_cmd if one doesn't use the default one? If there is not, it would be nice to add one.
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.
west_cmd
can be passed as a project option, we also had option to pass it as pytest argument before but since project api uses the west from ENV, we removed that
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.
@mehrdadh Thanks for this PR bumping Zephyr, that's great.
I did a first pass and left a few comments but I need more time to do the second pass.
Overall the Zephyr / Zephyr SDK looks good.
Question: do you really want to remove tests/micro/project_api/test_arduino_microtvm_api_server.py ? If so, could you elaborate a bit more on what you meant by "removed test since it was importing microtvm_api_server to tvm" ? Also, see my comments on touching Arduino code in this PR. I think ideally the Arduino parts should be left to a separate PR.
apps/microtvm/zephyr/template_project/launch_microtvm_api_server.sh
Outdated
Show resolved
Hide resolved
apps/microtvm/zephyr/template_project/launch_microtvm_api_server.sh
Outdated
Show resolved
Hide resolved
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.
Thanks @mehrdadh LGTM now 👍
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.
LGTM!
@alanmacd @mkatanbaf @guberti Thanks for the reviews. @mehrdadh Thanks a lot for the change. Kudos for the awesome PR description! :-) |
This PR updates the microTVM code to use Zephyr 3.2 and SDK 0.15.2. Az a result of this change, there are few other changes that are included in this PR:
west
is registered. However, for Arduino it uses a separate virtual ENV with python3 version that exists in the host.tests/micro/project_api/test_arduino_microtvm_api_server.py
since these tests where using arduino microtvm api server by importing it to TVM. We no longer support Arduino/Zephyr dependencies in TVM testing python ENV.There would be a follow up work to move zephyr to a completely separate virtual ENV.