-
Notifications
You must be signed in to change notification settings - Fork 37
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
Allow multiple simulated robots #18
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
except BaseException: | ||
pubsub = redis_client.pubsub() | ||
# see https://redis.io/topics/notifications | ||
pubsub.psubscribe(**{'__keyevent@0__:set': redis_change_handler}) |
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.
With more than one vehicles in Redis, some active and others inactive, it's critical to replace the previous /get_herd_data
polling with the new SocketIO based active notification. It's based on http://redis.io/topics/notifications: when a Redis key is set by zmq_server.py
, a new event is triggered and a message is broadcasted to all connected web clients. Inactive vehicles won't trigger such message hence not being shown up on the web UI.
Excited to test this! I will find some time next week to review. |
663697e
to
0f01a64
Compare
I am getting a few errors on this one.
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:
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:
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! |
You have to use The |
0f01a64
to
936838d
Compare
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. |
Fixed the |
2f7ad5a
to
bc5858d
Compare
bc5858d
to
5a1bca1
Compare
ac03432
to
e8ef5d5
Compare
Closes #14. Problems may surface when connecting real robots, but I don't expect many.
It also includes all previous pull requests: closes #12, closes #16, closes #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.