OpenAgricultureFoundation / openag_brain

ROS package for controlling an OpenAg food computer
GNU General Public License v3.0
221 stars 68 forks source link

[Exploration] Stateless Recipe Interpreter #254

Closed gordonbrander closed 7 years ago

gordonbrander commented 7 years ago

Rationale

The current recipe interpreter is rather complex... probably the most complex and difficult-to-test portion of code outside of the interface with rosserial.

If we make the interpreter a stateless function, it would be easy to test, and we could use the standard ROS event loop pattern for timing.

Details

Approach:

RecipeHandler becomes much simpler. It is just a threadsafe way to set and retrieve the document for the currently running recipe. It is responsible for:

Note that methods should be threadsafe because ROS services run in a separate thread.

cc @rbaynes @Spaghet

sp4ghet commented 7 years ago

I like it, allows for recipe scrubbing as mentioned in #79 much easier.

ghost commented 7 years ago

@gordonbrander I don't understand what you mean by a stateless interpreter. As I understand interpreters, maintaining state is an essential part of what they do (e.g. call stacks, variables, current instruction being executed, etc.).

Do you mean you want to write a compiler that translates recipe source code into an intermediate representation--maybe a list of scheduled actions for changing control parameters? If so, that's a great idea. You could unit test the scheduler, unit test the modules that implement the configurable control loops, and unit test that recipe source files produce the correct compiled output.

[edit: The mental model for this could be: A recipe is the source code for a food computer, your interpreter compiles the source into food computer "byte code", then the interpreter uses a food computer virtual machine (ROS, openag_brain, etc.) to execute the byte code.]

gordonbrander commented 7 years ago

@wsnook stateless in the sense that the recipe is read by a stateless function (https://github.com/OpenAgInitiative/openag_brain/blob/recipe_stateless/nodes/recipe_handler.py#L46), which is easy to test.

Previously, the recipe interpreter was a an iterator that made blocking calls to sleep.

ghost commented 7 years ago

Hmm... maybe I'm just getting tripped up by making assumptions that you're using the semantics I'm familiar with. I think of interpreter in the interpreter vs. compiler sense--like the python interpreter. It sounds like probably you mean something else by "recipe interpreter".

TL;DR never mind. I'll wait and watch.

gordonbrander commented 7 years ago

interpreter ~= "thing that reads the recipe file and turns it into setpoints over time"

rbaynes commented 7 years ago

Exploration done. Code implemented on branch stateless_recipe for this issue: https://github.com/OpenAgInitiative/openag_brain/pull/255

Will be merged into master once we have a few more tests and the whole team reviews it.