Twisted-Fields / acorn-precision-farming-rover

Source code for Acorn, the precision farming rover by Twisted Fields
Apache License 2.0
262 stars 37 forks source link

Allow multiple simulated robots #18

Open merlinran opened 2 years ago

merlinran commented 2 years ago

Closes https://github.com/Twisted-Fields/acorn-precision-farming-rover/issues/14. Problems may surface when connecting real robots, but I don't expect many.

It also includes all previous pull requests: closes https://github.com/Twisted-Fields/acorn-precision-farming-rover/pull/12, closes https://github.com/Twisted-Fields/acorn-precision-farming-rover/pull/16, closes https://github.com/Twisted-Fields/acorn-precision-farming-rover/pull/17.

ACORN_NAMES='a b c' make simulation would launch both the server and 3 vehicles named as a, b, and c accordingly, each be shown up on the web UI and controlled separately. Note the 5 Acorn icons shown on the screenshot. The docker containers eat up a lot of CPU, but fixable.

image
codecov-commenter commented 2 years ago

Codecov Report

Merging #18 (6718d41) into main (a35a238) will decrease coverage by 0.13%. The diff coverage is 64.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
- Coverage   45.31%   45.17%   -0.14%     
==========================================
  Files          20       20              
  Lines        2615     2612       -3     
==========================================
- Hits         1185     1180       -5     
- Misses       1430     1432       +2     
Impacted Files Coverage Δ
vehicle/remote_control_process.py 59.60% <0.00%> (ø)
vehicle/master_process.py 82.51% <64.28%> (-1.05%) :arrow_down:
vehicle/model.py 91.35% <100.00%> (ø)
vehicle/tests/test_main_process.py 93.18% <100.00%> (-0.16%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a35a238...6718d41. Read the comment docs.

tlalexander commented 2 years ago

Excited to test this! I will find some time next week to review.

tlalexander commented 2 years ago

I am getting a few errors on this one.

$ NAME=a docker-compose -f docker-compose-simulation.yml build
ERROR: The Compose file './docker-compose-simulation.yml' is invalid because:
Invalid service name 'acorn_vehicle_${NAME}' - only [a-zA-Z0-9\._\-] characters are allowed

So I am not sure how to build the containers for this one.

Maybe my existing containers are okay, but when I ran the code the vehicles did not show up, and when I attached to one I got a small error:

$ make attach-vehicle c
/bin/sh: 2: [[: not found
/home/pi #

Specifically "/bin/sh: 2: [[: not found" is an error I thought could be resolved if I build containers, but I wasn't able to sort that out.

I can't stop the containers, I get the same error:

$ make stop
/bin/sh: 2: [[: not found

All the containers remain running.

Looks like some user error on my part so any guidance appreciated. See also my note about make and whether it is necessary, since this PR relies on that.

Oh also the robot detail is over the top of the map view (well it was in the first UI PR, I can't tell yet on this one) but probably would be better if the robot detail was next to the map view as before?

Thanks for all this great work!

merlinran commented 2 years ago

You have to use ACORN_NAMES='a b c' make simulation (or simply make simulation if only one is needed) now since docker-compose-simulation.yml becomes a template to be able to launch more than one vehicle containers. The simulation make target loops over the names, generates a separate docker-compose file for each name and starts immediately. See https://github.com/Twisted-Fields/acorn-precision-farming-rover/pull/18/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R13-R15 . It adds complexity but is still the simplest way I can think of to be able to launch multiple vehicles.

The /bin/sh: 2: [[: not found error is something about the shell syntax. Which shell are you using on your Linux? I can make some changes to make them POSIX compatible, i.e., replace [[]] with [].

merlinran commented 2 years ago

As for the robot details, now with support to multiple robots, there's a chance that none of the robots are selected, which currently just hides the details and show the map in full window. If we want to keep the details section always visible, I think I can show something like "Please select a robot on the map to show details" when nothing is selected.

merlinran commented 2 years ago

Fixed the Makefile as well as the robot details. Kindly have a look again @tlalexander