-
Notifications
You must be signed in to change notification settings - Fork 4
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
[RSDK-8402] Migrate from OakCamera to DepthAI #51
Conversation
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.
LGTM. Nice work!
Just left a couple of questions to better understand things.
BGR color wonkiness ok? Or should we handle that better?
"oak.start() called before oak was assigned. Must configure worker first." | ||
async def start(self): | ||
self.device = None | ||
while not self.device and self.should_exec: |
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.
[q] will this infinite loop if device fails to initialize?
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.
Yes it will, but since it's an async function that async sleeps, it should yield to other tasks on the event loop
self.pipeline = dai.Pipeline() | ||
try: | ||
stage = "color" | ||
color_node = configure_color() |
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.
[q] Will there be any issues wrt early exit of the try block here if one of the nodes fails to initialize?
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.
Good point. I think we actually set configured to True after, which is wrong. I'm gonna change it to re-raise a helpful exception instead in the except block
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.
Done
src/components/worker/worker.py
Outdated
|
||
if self.cfg.sensors.stereo_pair: | ||
self.depth_queue = self.device.getOutputQueue( | ||
self.depth_stream_name, 30, blocking=False |
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.
[q] Is it ok to have hardcoded queue size here?
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.
Done (unhardcoded it and cleaned up some defaults at the top of the file)
BGR looking weird through App is expected. On the scope, Pete raised the point that on Tennibot's demo repo, they request BGR color order explicitly. The attribute is default rgb unless bgr is explicitly supplied though |
RSDK-8402
PR comment in luxonis/depthai
We were notified by Luxonis folk that they are sunsetting the OakCamera SDK. We should refactor our integration to use DepthAI
This PR switches out
OakCamera
functionality with the equivalent new idioms presented in their new shiny documentation https://docs.luxonis.com/I did more thorough manual testing since this is kind of a huge change.
Testing
OAK-D tests on Mac
Integration tests
Passing ✅
OAK-D config with "color" sensor (get_image stream on app)
✅
Reconfigure from above -> OAK-D config with "color" sensor and dimensions (get_image stream on app)
✅
Reconfigure from above -> OAK-D config add FPS (get_image stream on app)
Same as above but with choppy stream ✅
Reconfigure from above -> OAK-D config with color main sensor and depth secondary sensor (get_image stream on app)
Same as above ✅
I also switched the order and tested color as the main sensor with get_image and it looks good too ✅
Reconfigure from above -> OAK-D config with depth main sensor and color secondary sensor (get_image stream on app)
✅
OAK-D config with depth main sensor and color secondary sensor (get_images from my Python client)
Honestly in a future PR I should source control the Python test client. But the results look good!
✅
OAK-D config with depth main sensor and color secondary sensor (get_point_cloud from app)
OAK-FFC-3P on my RPI4
Single sensor on cam_c (get_image on app)
Hey Johnn ✅
Reconfigure from above -> two sensors with main on cam_b (get_image on app)
✅
Two sensors with main on cam_b (get_images with python test client)
Two similar images were
.show()
-ed successfully by pillow ✅Two sensors with main on cam_b with all attributes configured (get_image on app)
Color wonkiness of BGR is good. Frame rate looks around 5. Interleaved is not visually discernable, but no errors and we do set it! ✅