Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 0 additions & 55 deletions examples/lua/main.c

This file was deleted.

78 changes: 78 additions & 0 deletions examples/lua_REPL/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
APPLICATION = lua_repl

# If no BOARD is found in the environment, use this default:
BOARD ?= native

# This has to be the absolute path to the RIOT base directory:
RIOTBASE ?= $(CURDIR)/../..

BOARD_INSUFFICIENT_MEMORY := bluepill calliope-mini cc2650-launchpad \
cc2650stk maple-mini microbit nrf51dongle \
nucleo-f030r8 nucleo-f031k6 nucleo-f042k6 \
nucleo-f070rb nucleo-f072rb nucleo-f103rb \
nucleo-f302r8 nucleo-f303k8 nucleo-f334r8 \
nucleo-f410rb nucleo-l031k6 nucleo-l053r8 \
opencm904 spark-core stm32f0discovery \
stm32mindev airfy-beacon arduino-mkr1000 \
arduino-mkrfox1200 arduino-mkrzero arduino-zero \
b-l072z-lrwan1 cc2538dk ek-lm4f120xl feather-m0 \
ikea-tradfri limifrog-v1 mbed_lpc1768 nrf6310 \
nucleo-f091rc nucleo-l073rz nz32-sc151 \
openmote-cc2538 pba-d-01-kw2x remote-pa \
remote-reva remote-revb samd21-xpro saml21-xpro \
samr21-xpro seeeduino_arch-pro slstk3401a \
sltb001a slwstk6220a sodaq-autonomo \
sodaq-explorer stk3600 stm32f3discovery \
yunjia-nrf51822

BOARD_BLACKLIST := arduino-duemilanove arduino-mega2560 arduino-uno \
chronos hifive1 jiminy-mega256rfr2 mega-xplained mips-malta \
msb-430 msb-430h pic32-clicker pic32-wifire telosb \
waspmote-pro wsn430-v1_3b wsn430-v1_4 z1

# Comment this out to disable code in RIOT that does safety checking
# which is not needed in a production environment but helps in the
# development process:
DEVELHELP ?= 1

# Uncomment the following lines to enable debugging features.
#CFLAGS_OPT = -Og
Copy link
Contributor

Choose a reason for hiding this comment

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

This optimizes for debugging right? Just curious as to what exactly it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It tries to achieve a decent level of optimization, but it leaves out any optimization which could interfere with debugging. If you compile with -O2 and then try to debug, you get all kinds of weird jumps.

Copy link
Contributor

Choose a reason for hiding this comment

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

thx!

Copy link
Member

Choose a reason for hiding this comment

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

Is this only useful in this example, if not, would you mind reworking it to a PR to a more generic solution where it is for example enabled with a variable, similar how DEVELHELP enables a number of features at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to keep it separate. Running the full test suite requires -DLUA_DEBUG but that doesn't mean I want to compile with -Og because it may take too much flash.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like my comment was not completely clear here. What I meant is that to me, the -Og option is not only useful here, but could be useful for debugging other examples as well. So what I would like to propose is an option along the lines of the current DEVELHELP, maybe called OPTIMIZE_DEBUG to enable -Og for a random example.

#CFLAGS += -DDEBUG_ASSERT_VERBOSE -DLUA_DEBUG

# Change this to 0 show compiler invocation lines by default:
QUIET ?= 1

# This value is in excess because we are not sure of the exact requirements of
# lua (see the package's docs). It must be fixed in the future by taking
# appropriate measurements.
CFLAGS += -DTHREAD_STACKSIZE_MAIN='(THREAD_STACKSIZE_DEFAULT+7000)'

USEPKG += lua

include $(RIOTBASE)/Makefile.include

# The code below generates a header file from any .lua scripts in the
# example directory. The header file contains a byte array of the
# ASCII characters in the .lua script.

LUA_PATH := $(BINDIR)/lua

# add directory of generated *.lua.h files to include path
CFLAGS += -I$(LUA_PATH)

# generate .lua.h header files of .lua files
LUA = $(wildcard *.lua)

LUA_H := $(LUA:%.lua=$(LUA_PATH)/%.lua.h)

$(LUA_PATH)/:
@mkdir -p $@

# FIXME: This way of embedding lua code is not robust. A proper script will
# be included later.

$(LUA_H): | $(LUA_PATH)/
$(LUA_H): $(LUA_PATH)/%.lua.h: %.lua
xxd -i $< | sed 's/^unsigned/const unsigned/g' > $@

$(RIOTBUILD_CONFIG_HEADER_C): $(LUA_H)
32 changes: 32 additions & 0 deletions examples/lua_REPL/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## Lua interactive interpreter

### About
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide a readme with the available commands? I ran the example but don't know what to do :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest a link to the lua programming guide and a brief rundown of what's supported, in the package API reference, and then a note here linking to there?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jcarrano pointed out that it's not our place to teach people to program in Lua (we wouldn't link to a JavaScript tutorial in the jerryscript pkg for example). Worth noting that at the moment we haven't done any bindings. The only officially supported function is print() as far as I'm aware

Copy link
Contributor

@danpetry danpetry Jun 29, 2018

Choose a reason for hiding this comment

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

Link to the lua page and some brief info "works in the same way as the lua shell in the default installation" has been added


This example shows how to run a [Lua](https://www.lua.org/) Read-Eval-Print loop.
It works in a similar way to the lua shell that comes with your operating
system's default lua installation.


### How to run

Type `make all flash` to program your board. The lua interpreter communicates
via UART (like the shell).
Copy link
Member

Choose a reason for hiding this comment

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

small nitpick: "like the RIOT-os shell"


It is not recommended to use `make term` because the default RIOT terminal messes
up the input and output and the REPL needs multi-line input. Instead, use something
like `miniterm.py` from pyserial:
Copy link
Member

Choose a reason for hiding this comment

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

Is this something that should/could be fixed on the pyterm side? I think the pyterm also uses pyserial under the hood.


```
miniterm.py --eol LF --echo /dev/ttyACM0 115200
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, this seems to have limitations too, i.e. not being able to delete things? Could you document this and raise an issue/PR to fix in some way? IMO neither of these terminals provide a slick user experience. Would be good to move towards doing that, if we're going to say that we provide an interactive interpreter shell for Lua.
RE the shell implementation in general - we've talked about this in person, but would it be worth also (later) providing a Lua shell as a module? I think you said something about efficiency of this approach or something but can't remember the reasoning of this over a C implementation. In any case, this is a cool example and we could maybe put a module implementation on a roadmap or something for future development.

```

By default only some of the builtin modules are loaded, to preserve RAM. See
the definition of `BARE_MINIMUM_MODS` in main.c.

### Using the interpreter

See the [Lua manual](https://www.lua.org/manual/5.3/) for the syntax of the language.

Each piece of single or multi-line input is compiled as a chunk and run. For this
reason, issuing "local" definitions may not work as expected: the definitions
will be local to that chunk only.
63 changes: 63 additions & 0 deletions examples/lua_REPL/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

maybe this could be in the same lua example. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

(basically the LUA Repl implementation is running LUA. So, should be enough to show how it works)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the thing is the REPL requires more resources, so it runs on less boards (part of that is because I'm enabling the lua test module).
My idea was to have the lua example showcase the basic API, and another example with a more complex app.

Copy link
Member

Choose a reason for hiding this comment

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

check the OpenThread test at #7149. It's possible to switch between Full Thread Device and Minimum Thread Device with a Makefile flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting I test the type of device and run the REPL or the Hello World accordingly?

Copy link
Member

Choose a reason for hiding this comment

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

ok, let's leave it like that then. We can discuss it later in further PRs and don't block this one

Copy link
Member

Choose a reason for hiding this comment

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

(or what @jcarrano thinks the best)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for separate examples for clarity reasons. Could rename example/lua to example/lua_hello-world or something. In general I think it's more important that the examples are all concise, useful and non-overlapping than that they are simply small in number

Copy link
Member

Choose a reason for hiding this comment

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

I like @danpetry approach

Copy link
Contributor

Choose a reason for hiding this comment

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

This is corrected

* Copyright (C) 2018 Freie Universität Berlin.
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup examples
* @{
*
* @file
* @brief Lua shell in RIOT
*
* @author Juan Carrano <j.carrano@fu-berlin.de>
*
* @}
*/

#include <stdio.h>
#include <string.h>

#include "lua_run.h"
#include "lua_builtin.h"
#include "repl.lua.h"

/* The basic interpreter+repl needs about 13k ram AT Minimum but we need more
* memory in order to do interesting stuff.
*/
#define MAIN_LUA_MEM_SIZE (40000)
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to call it HEAP instead of MAIN?


static char lua_memory[MAIN_LUA_MEM_SIZE] __attribute__ ((aligned(__BIGGEST_ALIGNMENT__)));

#define BARE_MINIMUM_MODS (LUAR_LOAD_BASE | LUAR_LOAD_IO | LUAR_LOAD_CORO | LUAR_LOAD_PACKAGE)

const struct lua_riot_builtin_lua _lua_riot_builtin_lua_table[] = {
{ "repl", repl_lua, sizeof(repl_lua) }
};


const struct lua_riot_builtin_lua *const lua_riot_builtin_lua_table = _lua_riot_builtin_lua_table;

const size_t lua_riot_builtin_lua_table_len = 1;

int main(void)
{
printf("Using memory range for Lua heap: %p - %p, %zu bytes\n",
lua_memory, lua_memory + MAIN_LUA_MEM_SIZE, sizeof(void *));

while (1) {
int status, value;
puts("This is Lua: starting interactive session\n");

status = lua_riot_do_module("repl", lua_memory, MAIN_LUA_MEM_SIZE,
BARE_MINIMUM_MODS, &value);

printf("Exited. status: %s, return code %d\n", lua_riot_strerror(status),
value);
}

return 0;
}
94 changes: 94 additions & 0 deletions examples/lua_REPL/repl.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
--[[
Copy link
Contributor

Choose a reason for hiding this comment

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

As you mentioned it is probably not necessary to be thorough with the Lua style guide. One thing I did notice, though, is that indents are generally two spaces. This would also reduce the array size a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another note for the future: write a "partial minifier" that preserves line numbers and variable names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two-space indents make me want to rip off my eyes.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. That's a fair reaction tbh

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the copyright and license information is missing from this file.

@file repl.lua
@brief Read-eval-print loop for LUA
@author Juan Carrano <j.carrano@fu-berlin.de>
Copyright (C) 2018 Freie Universität Berlin. Distributed under the GNU Lesser General Public License v2.1.
]]

local _R_EVAL = 0
local _R_CONT = 1
local _R_EXIT = 2
local _R_ERROR = 3

--[[ Read code from standard input (whatever stdin means for lua)
@return action_code what the eval loop should do
@return code_or_msg either code (_R_EVAL) or a message (_R_ERROR) or nil
(_R_CONT, _R_EXIT).
]]

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you make a few changes to make this easier to understand out-of-the-box - i.e. so that someone can come in with no help and learn a few lua features? Suggestions are:

  • rename "maybe_code" to something like "is_executable_code" or something and explain that the three if maybe_code == nil sections check whether it's an expression, then a statement, then a line from a multiline
  • briefly explain the coroutine functionality
  • explain what the typical dual return codes for lua are for
  • maybe explain that load() compiles chunks line by line and then pcall() executes the compiled bytecode

I understand that comments come at a memory cost since it's all being parsed into the array. Would be good to find a good balance here. I think if you can go as far as possible with readable code itself that would be good, by renaming some of the variables. Potentially, could also write some notes in the readme?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's poorly documented. However, teaching Lua is beyond the scope of our documentation. I would expect people who want to use lua to learn the core language elsewhere, and in RIOT we can limit ourselves to document only the RIOT-specific extensions and usage.

Copy link
Contributor

@danpetry danpetry Jun 26, 2018

Choose a reason for hiding this comment

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

ok cool, thanks for making the changes, they seem sufficient.

local function re()
io.write("L> ")
io.flush()
local ln = io.read()

if not ln then
return _R_EXIT
elseif ln == "\n" then
return _R_CONT
end
-- Try to see if we have an expression
local maybe_code, compile_err = load("return "..ln)
-- Try to see if we have a single-line statement
if not maybe_code then
maybe_code, compile_err = load(ln)
end
-- Try a multiline statement
if not maybe_code then
-- It's not really necessary to use a coroutine, but it shows that they
-- work.
local _get_multiline = coroutine.create(
function ()
coroutine.yield(ln.."\n") -- We already have the first line of input
while 1 do
io.write("L.. ")
io.flush()
local l = io.read()
if #l ~= 0 then
l = l.."\n"
end
coroutine.yield(l)
end
end
)
local get_multiline = function()
local a, b = coroutine.resume(_get_multiline)
if a then
return b
else
return nil
end
end

maybe_code, compile_err = load(get_multiline)
end

if not maybe_code then
return _R_ERROR, compile_err
else
return _R_EVAL, maybe_code
end
end

local function repl()
io.write("Welcome to the interactive interpreter\n");

while 1 do
local action, fn_or_message = re()

if action == _R_EVAL then
local success, msg_or_ret = pcall(fn_or_message)
if not success then
print("Runtime error", msg_or_ret)
elseif msg_or_ret ~= nil then
print(msg_or_ret)
end
elseif action == _R_EXIT then
print()
return
elseif action == _R_ERROR then
print("Compile error:", fn_or_message)
end -- (action == _R_CONT) : do nothing
end
end

repl()
8 changes: 3 additions & 5 deletions examples/lua/Makefile → examples/lua_basic/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
APPLICATION = lua
APPLICATION = lua_basic

# If no BOARD is found in the environment, use this default:
BOARD ?= native
Expand Down Expand Up @@ -29,12 +29,10 @@ DEVELHELP ?= 1
# Change this to 0 show compiler invocation lines by default:
QUIET ?= 1

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please rename this example to lua_hello-world or something similar, as discussed below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I'm thinking "lua-basic".

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

has been changed

USEMODULE += ps

ifneq ($(BOARD),native)
# This stack size is large enough to run Lua print() functions of
# various lengths. Other functions untested.
CFLAGS += -DTHREAD_STACKSIZE_MAIN=4096
CFLAGS += -DTHREAD_STACKSIZE_MAIN='(THREAD_STACKSIZE_DEFAULT+2048)'
endif

USEPKG += lua
Expand All @@ -61,6 +59,6 @@ $(LUA_PATH)/:
$(LUA_H): | $(LUA_PATH)/
$(LUA_H): $(LUA_PATH)/%.lua.h: %.lua

xxd -i $< | sed 's/^unsigned/const/g' > $@
xxd -i $< | sed 's/^unsigned/const unsigned/g' > $@

$(RIOTBUILD_CONFIG_HEADER_C): $(LUA_H)
File renamed without changes.
Loading