-
Notifications
You must be signed in to change notification settings - Fork 683
targets/zephyr: Update to new Jerry API and improve REPL interaction. #1190
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
targets/zephyr: Update to new Jerry API and improve REPL interaction. #1190
Conversation
This follows Unix port (main-unix.c) and API example doc. |
@sergioamr, @poussa: FYI. |
Sounds like a good idea. The run_simple initializes and shuts down the engine, so it cannot return with any meaningful value, since jerry heap has been freed already. |
LGTM |
1 similar comment
LGTM |
@LaszloLango : If #1177 is close to be merged (it seems to be), I guess it makes sense to land it first, then I'll update this patch. Thanks. |
LGTM. I would like to be able to have the Travis integration working, so changes on the API get highlighted and do not break the build. The situation is that @Akromx16 will go back to university really soon and i am not sure if he would be able to finish his integration on #1144 Could someone with travis experience help on that PR? |
#1177 is merged. |
@LaszloLango : Thanks, refactored to the new API, rebased to master as of now. |
LGTM |
@@ -158,6 +158,7 @@ const struct shell_cmd commands[] = | |||
void main (void) | |||
{ | |||
printf ("Jerry Compilation " __DATE__ " " __TIME__ "\n"); | |||
jerry_init (JERRY_INIT_EMPTY); | |||
shell_register_app_cmd_handler (shell_cmd_handler); | |||
shell_init ("js> ", commands); | |||
} /* main */ |
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.
jerry_cleanup
should be called before the end of main
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.
@LaszloLango : That's not the case with Zephyr. Generally, a typical embedded app would work as an infinite loop (it "exits" via hardware reset), and specifically with Zephyr, shell_init() just sets up background task and exit, all actual input passed to callbacks at later time. So, it's not ok to deinitialize Jerry after call to shell_init(), and per above, no need to deinitialize at all. Hope that makes sense. I'm adding a comment to the patch with this info.
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.
shell_init will spawn a fiber that performs the shell task.
task_fiber_start(stack, STACKSIZE, shell, 0, 0, 7, 0);
If you clean zephyr there it will just crash.
With the change of running code on eval i think pfalcon is right on not cleaning up after execution.
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.
@pfalcon, @sergioamr, OK, thanks for the explanation.
LGTM after |
lgtm |
Various calls updated to use API from jerry-api.h (to-be JerryScript 1.0 API). Use jerry_eval() instead of jerry_run_simple(), as it allow to store (and then access) variables in a global environment. E.g. following now works (entered and executed as two separate lines): a = 1 print(a) JerryScript-DCO-1.0-Signed-off-by: Paul Sokolovsky [email protected]
Added comment to the source why jerry_cleanup() is not called, to address @LaszloLango's comment above. |
LGTM |
Using jerry_eval() instead of jerry_run_simple() allows to store (and then access)
variables in a global environment. E.g. following now works:
JerryScript-DCO-1.0-Signed-off-by: Paul Sokolovsky [email protected]