961 protect from empty camera image #967
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- prior code just waited a couple of seconds. - this new code now starts asking for frames and will wait until a frame is return or a timeout happens - Generally this is a more reliable and faster way to warm the camera; if it is going to work it will return a frame quickly. So start up is faster. - The warmup code also declares a better error if the camera can't start; it raises the error in the camera part rather than downstream where the image is used or later in the pipeline. That makes it easier to diagnose what is going on.
- if the camera returns a None image the code would crash. That could happen with the prior CISC camera if it did not start properly. However, that error was confusing. - Now this code will simply ignore an None image.
- added --debug flag. if this is passed to 'drive' command then debug window is shown. Otherwise it does not. - this fixes an issue when using a headless OS on the donkeycar; open cv cannot create a debug window and so crashes.
- this guarantees we can produce frames before the camera is used - also detects if pygame is not installed and prints instructions
- if both a CSIC camera and a USB camera are connected then there will be more than one camera available - The current code always just choose index 0 - This change allows the index to be chose with cfg.CAMERA_INDEX, which defaults to zero
This was referenced Dec 17, 2021
DocGarbanzo
reviewed
Dec 18, 2021
donkeycar/parts/camera.py
Outdated
| print('PiCamera loaded.. .warming camera') | ||
| time.sleep(2) | ||
| # get the first frame or timeout | ||
| print('PiCamera loaded...') |
There was a problem hiding this comment.
Can you convert your print into logger.info?
donkeycar/parts/camera.py
Outdated
| print('PiCamera opened.. .warming camera') | ||
| warming_time = time.time() + 5 # quick after 5 seconds | ||
| while self.frame is None and time.time() < warming_time: | ||
| print(".", end="") |
There was a problem hiding this comment.
Not sure how to do this w/ logging though...
There was a problem hiding this comment.
... actually I figured out a way to do it like this:
import logging
import time
# handler = logging.StreamHandler()
dfmt = "%d %b %Y %H:%M:%S"
format = '%(asctime)s %(message)s'
logging.basicConfig(format=format, datefmt=dfmt, level='INFO')
logger = logging.getLogger(__name__)
handler = logging.StreamHandler()
if __name__ == "__main__":
logger.info('Starting')
logger.propagate = False
logger.addHandler(handler)
handler.terminator = ""
for i in range(20):
logger.info(f'-' if i % 2 == 0 else '>')
time.sleep(0.3)
logger.info('\n')
logger.removeHandler(handler)
logger.propagate = True
logger.info('bye')There was a problem hiding this comment.
didn't see this. I've made a change that is satisfactory I think.
|
Also added support for CAMERA_INDEX config when using CVCAM |
DocGarbanzo
reviewed
Dec 21, 2021
| import os | ||
| import time | ||
| import numpy as np | ||
| from PIL import Image | ||
| import glob | ||
| from donkeycar.utils import rgb2gray | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| logging.basicConfig(level=logging.INFO) |
There was a problem hiding this comment.
We are already setting the logging level in the car app (in complete.py) to INFO already. Because this goes to the root logger, you would optimally set this only once where you start the application. There is no harm with this line, but if we decide to dynamically change the logger level per argument or config then we have to find and remove all these local settings as they might interfere/overwrite the application settings.
DocGarbanzo
approved these changes
Dec 21, 2021
DocGarbanzo
requested changes
Dec 21, 2021
DocGarbanzo
approved these changes
Dec 23, 2021
There was a problem hiding this comment.
As discussed with @Ezward - squash merging should be fine here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses Issue 961 and a little more. Basically our cameras could return None images if they did not start correctly. The warmup was just a sleep(2) and no check was actually made to know that the camera started. This could result in hard to understand downstream errors.