Remote control car example with OpenCV UI #46
Conversation
… the camera feed, using OpenCV's UI.
…d with a kernel and a 2D filter.
|
I added the image resize and the sharpening for two main reasons:
|
There was a problem hiding this comment.
I just tried this and it was super fun! Great work :)
Just a thing that I thought could make driving a bit more intuitive. I think you could change the keys to make the robot run only while pressing certain keys, you could implement something like this:
- Have 'w', 'a', 's' and 'd' control the robot movement while pressed. You don't need to send the motor commands every time, just whenever a new key is pressed or unpressed (you can calculate this on each loop: if no keys are pressed, I believe the value of
keywill be None and you can stop all motors). - Then, have two extra keys to change the linear and angular speed of the robot, like you are doing now. This way you can control the speed of the robot, and also move it using the direction keys. If you want to get fancy, you could also add a couple of trackbars so that you can visualize the current speed (and control it with the mouse too).
Another thing you could try is to use pynput instead of OpenCV's getkey to be able to detect multiple key press and releases. It might simplify things a bit actually.
Let me know what you think and if you need help ;)
Edit:
I forgot to mention: you could convert the key to a char to do all key checks instead of converting all characters. That's going to look nicer and will be a lot less operations: you only do one translation when getting the key instead of several on each if statement.
examples/opencv_rc.py
Outdated
| # Exit the program | ||
| break | ||
|
|
||
| # Losing a bit of computational time to prevent sending motor |
There was a problem hiding this comment.
I don't think you are missing any computational time anywhere, just implementing a feature!
(I just wanted to say that there is no loss here that I can see at least, just fine code ;))
examples/opencv_rc.py
Outdated
| cli.add_handler(pc.event.EvtNewRawCameraImage, on_camera_img) | ||
|
|
||
| # Handle cliff and pick-up detection | ||
| cli.add_handler(pc.event.EvtCliffDetectedChange, stop_all) |
examples/opencv_rc.py
Outdated
| HEAD_LIGHT), end='\r') | ||
| finally: | ||
| # This is to make sure that whatever happens during execution, the | ||
| # robot will always stop driving before exiting |
|
@gimait Thanks a lot for the feedback. |
|
Cool! Looking forward to see what you come up with. Hope it works with pynput, I have only used another package, I'll have a look at #47, we were planning on implementing object recognition features little by little, so it's great that you already implemented something for face recognition. We can talk about it over the ticket. |
|
@gimait So yeah, I am sorry for the 1 commit for all, but after your feedback I have been forced to reorganize my code into a class, which means basically rewriting everything. |
…() function, putting its content in the rawCameraImage callback, and replacing step() by a simple sleep in the main loop.
…al between frame display.
…in loop from blocking the execution of other threads while it is asleep.
|
After some more reflection on the laggy video feed, it seems pretty clear that the GIL is to blame for that (unless I have overlooked something). This brings the question: are we tied to only using threads or can we go multi-process? The idea would be to dedicate a single and simple process to displaying the video, while leaving the rest of the code intact. Any thoughts on that? |
There was a problem hiding this comment.
I looked a bit around and I have a couple of comments.
-
About the lagging and the multithreading: You are using event callbacks in your application, so I don't see where you are having issues with multithreading. Actually, you had it running nicely before with one thread, I think you should come back to that (using a queue to store new frames and displaying them on the screen in the main thread, no need for anything fancy). If it was working fine before, it will work again ;)
-
I like that you move things into a class, but I believe you are putting too much functionality in one place and this is leading to some confusion. I think you should separate the code in more objects. For example, use a class for the controls/key handling and a separate one to manage and display the images. If you then want to encapsulate everything in one class, create a third one that uses these two. Give it a thought and let me know if you need help structuring it. If this works nicely we could add it as part of the package and not just as example code.
I think you need to work this out a little more. It's looking nice, but we need to figure out what went wrong with the lagging and that.
examples/opencv_rc.py
Outdated
|
|
||
|
|
||
| # Declare a flag telling the program's main loop to stop | ||
| GO_ON = True |
There was a problem hiding this comment.
Why not declaring this as member of OpencvRC?
There was a problem hiding this comment.
To be honest, there is no reasons for that to be here. Some oversight on my end. Since I have to rework the class structure anyway, I will be sure to put it where it needs be.
|
|
||
| # Start the keyboard event listener | ||
| self._kbd_listener.start() | ||
| self._kbd_listener.wait() |
There was a problem hiding this comment.
Won't this wait statement stop the main thread here?
There was a problem hiding this comment.
This is a bit confusing indeed, but according to the documentation this will only stop the main thread until the listener is ready to do its job. In essence, it has the same purpose as the self._cozmo_clt.wait_for_robot() function above.
There was a problem hiding this comment.
Sounds good then! I wasn't sure so I thought I'd like to ask ;)
examples/opencv_rc.py
Outdated
| # This might seem odd, but is actually required by OpenCV to perform GUI | ||
| # housekeeping. See OpenCV's documentation for imshow() for more | ||
| # "in-depth" information. | ||
| cv.waitKey(25) |
There was a problem hiding this comment.
_on_new_image is being called whenever a new frame is available. The frames are streamed as recorded, which means that you don't need to wait here. I don't think the problem is here, but the lagging issues you are seeing could be related to this.
There was a problem hiding this comment.
As I mention in the comment on line 205-207, this wait is actually required by OpenCV. As per the documentation a call to cv.imshow() should always be followed by a call to cv.waitKey(), otherwise the image will not be displayed. The best I can do here is to set the wait to only 1 ms. But as you suggested in your overall comment, I will try putting the actual display function back into the main thread and have it communicate via queue with the callback.
Edit: As an addendum, I should specify that simply using time.sleep() or a blocking call to Queue.get() instead of cv.waitKey() does not work. If my understanding is correct, it seems that it is cv.waitKey() that is doing the actual work of displaying the image. cv.imshow() is only fast because it does not do much.
There was a problem hiding this comment.
Yes you are right, it might be causing trouble here because of the callback though. I think this should go separately from the frame retrieval
examples/opencv_rc.py
Outdated
| # Set both actions to NONE, which will stop the motors | ||
| self._set_action(Direction.NONE, Direction.NONE) | ||
|
|
||
| def _on_head_tilt_change(self, value): |
There was a problem hiding this comment.
I would suggest sending the command for the new head tilt whenever this changes. Otherwise, it will only be applied when you attempt to move the robot.
There was a problem hiding this comment.
I am not sure I understand the problem here. Within the _on_head_tilt_change() function I assign the new value for the head tilt using self.head_tilt = value. This in fact calls the property setter that I defined on line 450. This setter in turn sends an immediate command to Cozmo to move its head to the new angle. Maybe not the most straight forward way of doing this, but it works.
There was a problem hiding this comment.
😱 you are right! I completely missed the setters! good work with that, keep it!
examples/opencv_rc.py
Outdated
| self.head_tilt = value * pc.MAX_HEAD_ANGLE.radians + \ | ||
| (1 - value) * pc.MIN_HEAD_ANGLE.radians | ||
|
|
||
| def _on_lift_height_change(self, value): |
There was a problem hiding this comment.
The same comment as above applies here. I am using a property setter to make sure the new value stays within Cozmo's physical constraints and send a command for Cozmo to move its lift to new heights. The code corresponding to the property setter is located on line 469.
examples/opencv_rc.py
Outdated
| self._velocity['angular'] = max(0, | ||
| min(value, pc.MAX_WHEEL_SPEED.mmps / | ||
| pc.TRACK_WIDTH.mm)) |
There was a problem hiding this comment.
Just so you know, we are using 120 characters per line in pycozmo.
I don't know if it's very important but I personally like better longer lines ;)
If you want, you can check the flake8 configuration
There was a problem hiding this comment.
Thank you for pointing that out, I was not aware of it. I thought I was stuck with 80 characters per line.
I 100% agree with you on longer lines. Especially, on more modern screens where there is so much space. I will configure my IDE to put the cut of at 120 characters from now on (that is going to be liberating).
examples/opencv_rc.py
Outdated
|
|
||
| try: | ||
| # Instantiate a dummy event | ||
| evt = Event() |
There was a problem hiding this comment.
I don't think you need this. You are creating an event to do the work of sleep. I think you can do better ;)
There was a problem hiding this comment.
You are right. If I put the display function back into the main thread, this will be useless. I did not like it anyway, so good ridance.
|
Once again thanks a lot for your overall (and more specific) feedback on this new implementation. |
…ht into a separate class.
|
@gimait As mentioned in my previous comment I have simply split the existing code into three classes (and fixed the issues discussed above as well). I am still open to suggestion for different structuring, but I wanted to test if putting the display process back into the main thread would fix the video lag issue. Well it did not, so that's that. More digging is required to solve this particular problem. The rest, though, works like a charm. As always any feedback is welcome. |
|
@gimait eureka! I have found why the video was laggy. I do not really have the full explanation, but here is the main idea. Basically, the program uses
To be honest I do not like solution 1, but, and that is where things get ironical, it just so happen that under Linux (more specifically within a Xorg environment) passing |
|
Oh, I see. Of course, these two are fighting to get the keys. Have you thought of using a different method to display the video? From a quick search, I found you can probably do it with matplotlib or tkinter, but there might be other options. I looked into whether you could do it directly with PIL, but I don't think it's possible, unfortunately. I think any keyboard library will interfere with waitkey. It is quite a bad design decision from opencv to have only that function as an event manager. |
… the video or other issues related to key events.
|
@gimait Well OpenCV's documentation strikes again. To avoid any problems and the "requirement" of calling |
…ch state it is, rather than using a key press.
examples/opencv_rc.py
Outdated
| """ | ||
| Create the window OpenCV will use to display the different track bars, as well as start the keyboard listener. | ||
| :return: None | ||
| """ | ||
|
|
||
| # Create a new thread for the window containing the task bars to prevent any lag issues due to key events | ||
| cv.startWindowThread() | ||
| # Create a window for the control panel | ||
| cv.namedWindow(self._win_name) | ||
|
|
||
| # Create the different trackbars controlling the robot's velocities, | ||
| # head tilt, and lift height | ||
| cv.createTrackbar('Linear velocity', self._win_name, 0, 100, self._on_linear_velocity_change) | ||
| cv.createTrackbar('Angular velocity', self._win_name, 0, 100, self._on_angular_velocity_change) | ||
| cv.createTrackbar('Head tilt', self._win_name, 0, 100, self._on_head_tilt_change) | ||
| cv.createTrackbar('Lift height', self._win_name, 0, 100, self._on_lift_height_change) | ||
| cv.createTrackbar('Head light', self._win_name, 0, 1, self._on_head_light_change) |
There was a problem hiding this comment.
All the display code should be in the Display class.
You can pass references to the controller's change callbacks.
There was a problem hiding this comment.
I put that code here since in my mind it was not really part of the display per se. However, it can easily be moved in the Display class as you said. I will have a look at that later today.
There was a problem hiding this comment.
As you suggested I have moved the window and track bars creation into the display class. The RemoteController's callbacks are passed as argument to the Display.init() function. It is not pretty but it works, and it is more flexible that way.
There was a problem hiding this comment.
I put that code here since in my mind it was not really part of the display per see.
But this is what creates the display window and the sliders in it, without this you don't have a display, right? :) I think it's better to have it separate from the controls, what if you'd like to use another interface for the video streaming instead of opencv?
|
That's the irony of this situation. Everywhere in the documentation you are being told that |
…s references to callbacks in the RemoteController class.
|
I think it's good now, let's see if we can have it merged soon. @zayfod, when you have time, could you have a look and see what you think about the example? |
|
Hello all. Starting to work on pycozmo. I wanted to play a bit with your script but I had nochance nor under windows nor under linux Just hoping this ring some bells. |
|
Have you tried to get an image from Cozmo's camera by other means? |
|
Thanks for your answer and issues section opening. |
A simple script showing how to use OpenCV to remotely control Cozmo, while also streaming its camera feed in either color or gray scale. This is linked to issue #37.