Skip to content

Conversation

@samg9
Copy link

@samg9 samg9 commented Jan 17, 2020

This closes https://github.com/uvic-aero/onboard-computer/issues/39

Needs tests for pycam portion

Copy link
Contributor

@avesk avesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, should have been a bit more clear. What I was looking for here was a camera class that contains a method capture() which conditionally uses the piCam, or webcam (depending on whether the piCam is connected) to capture a single frame and return a numpy img object.

Copy link
Contributor

@avesk avesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Few more pieces of feedback

if found_picamera:
# initialize the camera and grab a reference to the raw camera capture
self.picamera = PiCamera()
self.picamera.resolution = (3280, 2464)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey can we make the resolution a class property? And set the default to (2592 , 1944)? I would say make it a constructor value, but use the default if no value is provided

Copy link
Contributor

@dragonprevost dragonprevost Feb 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @avesk , is there a reason you want to keep the resolution lower than 8mp? Is that more suitable for your classifier?

# initialize the camera and grab a reference to the raw camera capture
self.picamera = PiCamera()
self.picamera.resolution = (3280, 2464)
self.picamera.framerate = 32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would also be best stored as a class prop

self.picamera.framerate = 32
rawCapture = PiRGBArray(self.picamera, size=(640, 480))
self.camera_feed = camera.capture()
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove this call to pass now too

self.picamera = PiCamera()
self.picamera.resolution = (3280, 2464)
self.picamera.framerate = 32
rawCapture = PiRGBArray(self.picamera, size=(640, 480))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? I dont see it used anywhere

def capture(self):
cam = cv2.VideoCapture(self.camera_feed)
ret_val, frame = cam.read()
return cv2.imshow('my webcam', frame)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll wanna return the frame here. We'll have to come up with a different method for testing, probably write a test script that instantiates this class, calls capture() and then writes it to disk so that we can check that its coming in correctly

ret_val, frame = cam.read()
return cv2.imshow('my webcam', frame)

if __name__ == "__main__":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is just for testing?

@dragonprevost
Copy link
Contributor

dragonprevost commented Feb 3, 2020

Hey guys, Im a little late on the ball for this one but I feel that this has mostly been implemented in the PiCam module. It seems this is issue is almost complete already... but to keep client code working maybe we should try and port the code from this Capture file over to the PiCam module?

@avesk
Copy link
Contributor

avesk commented Feb 3, 2020

Hey guys, Im a little late on the ball for this one but I feel that this has mostly been implemented in the PiCam module. It seems this is issue is almost complete already... but to keep client code working maybe we should try and port the code from this Capture file over to the PiCam module?

sounds good, let's move the changes into the PiCam app then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants