-
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] Add microTVM Template Projects to tlcpack pip Package #9309
Conversation
class MicroTVMTemplateProject(enum.Enum): | ||
ZEPHYR = "zephyr" | ||
ARDUINO = "arduino" | ||
CRT = "crt" |
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.
Hi @mehrdadh Could you please elaborate why CRT is considered to be a Template Project?
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.
It's located here. We used this CRT host template project for CRT tests and also tutorials.
@junrushao1994 this PR is a step towards adding microtvm project API to pip package. Please take a look when you have time. thanks! |
@leandron just a friendly reminder about this PR. thanks! |
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
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 a couple comments, can address in a follow-on if they merit changes
list( | ||
APPEND | ||
ARDUINO_FILE_COPY_JOBS | ||
"apps/microtvm/arduino/template_project microtvm_api_server.py -> arduino" |
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.
should we not copy the whole template_project?
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.
we could. the only file that we don't need there is the README file.
version_output.replace("\n", "").replace("\r", "").replace(":", "").lower().split(" ") | ||
) | ||
full_version = version_output[version_output.index("version") + 1].split(".") | ||
full_version = re.findall("version: ([\.0-9]*)", version_output.lower()) |
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.
nit: should add r" string prefix
(see https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals regarding backslashes)
Sorry I merged it because I thought it was only pending on me. Can we push fixes on follow up PRs? |
@leandron I will send a patch. |
…che#9309) * zephyr lib fixed * restructure * readme * add arduino * add project template to setup.py * fix lint * fix tutorial * address comments from PR9274
…che#9309) * zephyr lib fixed * restructure * readme * add arduino * add project template to setup.py * fix lint * fix tutorial * address comments from PR9274
This PR:
This PR is currently waiting on #9274 and #9289 to merge.
cc @areusch @leandron