Skip to content

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

Merged
merged 1 commit into from
Jul 12, 2016
Merged

targets/zephyr: Update to new Jerry API and improve REPL interaction. #1190

merged 1 commit into from
Jul 12, 2016

Conversation

pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Jul 7, 2016

Using jerry_eval() instead of jerry_run_simple() allows to store (and then access)
variables in a global environment. E.g. following now works:

a = 1
print(a)

JerryScript-DCO-1.0-Signed-off-by: Paul Sokolovsky [email protected]

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 7, 2016

This follows Unix port (main-unix.c) and API example doc.

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 7, 2016

@sergioamr, @poussa: FYI.

@zherczeg
Copy link
Member

zherczeg commented Jul 8, 2016

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.

@zherczeg
Copy link
Member

zherczeg commented Jul 8, 2016

LGTM

1 similar comment
@poussa
Copy link
Contributor

poussa commented Jul 8, 2016

LGTM

@LaszloLango
Copy link
Contributor

This won't work after #1177
@pfalcon, would you like to land it now or wait for the API update first?

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 8, 2016

@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.

@pfalcon pfalcon changed the title targets/zephyr: Use jerry_eval() to provider better REPL (interactive prompt). targets/zephyr: Use jerry_eval() to provide better REPL (interactive prompt). Jul 8, 2016
@sergioamr
Copy link
Contributor

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?

@LaszloLango
Copy link
Contributor

#1177 is merged. docs/02.API-EXAMPLES.md is up to date, but docs/02.API-REFERENCE.md is not. Let me know, if you need any help with the update.

@LaszloLango LaszloLango added the jerry-port Related to the port API or the default port implementation label Jul 11, 2016
@pfalcon pfalcon changed the title targets/zephyr: Use jerry_eval() to provide better REPL (interactive prompt). targets/zephyr: Update to new Jerry API and improve REPL interaction. Jul 11, 2016
@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 11, 2016

@LaszloLango : Thanks, refactored to the new API, rebased to master as of now.

@zherczeg
Copy link
Member

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 */
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@LaszloLango

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.

Copy link
Contributor

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.

@LaszloLango
Copy link
Contributor

LGTM after jerry_cleanup call is added.

@sergioamr
Copy link
Contributor

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]
@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 12, 2016

Added comment to the source why jerry_cleanup() is not called, to address @LaszloLango's comment above.

@LaszloLango
Copy link
Contributor

LGTM

@LaszloLango LaszloLango merged commit 6b60e22 into jerryscript-project:master Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jerry-port Related to the port API or the default port implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants