Add functions to save image directly from the library #18
Conversation
Added a save_img function and take_and_save function to the library, and updated the examples to match. This puts all the saving right into the library, as it was the same across all platforms. Should work just fine with any existing code. But new code can just use a single function to take *and* save a picture!
"board" doesn't need to be imported for PC/Pi versions. Also left a note for modifying serial ports for PC version. This will require some minor changes to the CP part of the learn documentation.
Condensed all the switcher stuff into a single switcher.
Allows for even simpler image taking.
| Please checkout this PR where we're discussing a camera API: adafruit/circuitpython#3369 It'd be good to have the same API for this. (I'd love your feedback too since you are currently using the camera.) Thanks! |
| I will have a look in a bit! A unified camera deal would be great, but a lot this sort of stuff is out of my league. |
| Implementing such an API is, well. At like, floor 20, there's no elevators, and I really prefer to be in the park and occasionally poke my head in the lobby for a snack. To put in other terms: It requires a lot of knowledge I don't have and, to be honest, don't particularly care to learn. I don't like programming much. It seems like a really cool idea, there's some camera-specific issues maybe unless I missed things, but it will not be I that implements it here right now. |
| I don't expect you to change the whole api. Instead I'd like |
| The only reason I didn't use |
| Well. I think no issues. |
Ah, bummer! I think we should just hold off on unifying the API. We can merge this as-is and follow up with a big restructure when we want them to be the same. |
| I could potentially restructure take_and_save to follow the same form, but still he called take_and_save (or something better), to make unification easier when it does happen? |
| @Andon-A Ya, making it take the same arguments as the future take_picture could help. We could even add a comment to |
| Further thinking on this: While we could use various methods to make Also, the major thing I am not sure of is the buffer. What exactly is it supposed to be - A variable that's been set aside for use? I THINK the M4 boards have enough ram for this camera but larger images definitely not, and it would infringe on other things so I'm not sure. |
| Oh, also. Would you be available this weekend sometime when I actually go and poke at this? I can hit you up on Discord or wherever for specific things |
That's one way to go. I tend to replace things more than I should. The only thing to consider is the code size of having two different implementations to store in memory. We could split them into two files though.
I was thinking it could either be an in-memory buffer, like a bytearray, or a stream, like a file. The latter wouldn't need everything in memory at once.
I'm around. Let's coordinate a time. I try not to be on the computer a ton but can be if we coordinate. |
| I'm looking at this now, and if it's going to be like you mentioned here, the only thing I'm not sure of how to handle is the format. The VC0706 only returns a jpg, so I want to make it yell if you try something else. But I'm not sure what to check against for that, especially without having the camera module opened as well. Feel free to poke me here or on Discord whenever's handy for you, I'm basically going to be here all weekend |
Split into a legacy (doesn't break code) class and a VC0706Camera class that is intended to be in line with the camera API. Some tweaks to make Pylint happy. Left in the save_image bit to the legacy version, so there's an easy way to save files there (Although take_and_save is gone). Camera version doesn't use fileformat currently because I'm not sure how it should be checked. It's not super important since the camera only suports one. I've had some issues changing between image resolutions on the camera, but that's likely something specifically with the camera.
Both classes: Adds a reset() function, which resets the camera and resets the baudrate to whatever it was previously. This seems to be required in order to change the camera image size on the one I have, at least. This required adding an internal set_baudrate. External: Added a get_image_size which converts width and height arguments into the proper const. Added some retries for reading the buffer for VC0706Camera's take_picture. Added the "Failed to halt frame!" to the stop_video instead of having it external. Still need to handle that filetype.
| So the updates have done largely what I've said I was going to do. Still not sure how to handle that format, but otherwise it works. Had to fiddle with a few other things to get stuff to work well, but it's all in the commit notes. Examples need to be updated still. And the learn guide will need an update, too. |
Added a get_image_size to the VC0706Camera class, that returns (Width, Height) tuple. Updated the examples to use the VC0706Camera class. The filetype will default to JPG so they shouldn't need to be updated once that's done (But we'll see)
Stripped out additions to VC0706 class, changed VC0706Camera to Camera, used "with" with examples, changed sizes to a dict use, Removed extraneous file.
Issues with file saves meant this never got into the prior committ. Removes new things from VC0706 class. Changes get_image_size to a dict Changes VC0706Camera class to just Camera
| def set_baudrate(self, baud): | ||
| """As above. For internal use.""" | ||
| divider = None | ||
| if baud == 9600: | ||
| divider = _BAUDRATE_9600 | ||
| elif baud == 19200: | ||
| divider = _BAUDRATE_19200 | ||
| elif baud == 38400: | ||
| divider = _BAUDRATE_38400 | ||
| elif baud == 57600: | ||
| divider = _BAUDRATE_57600 | ||
| elif baud == 115200: | ||
| divider = _BAUDRATE_115200 | ||
| else: | ||
| raise ValueError("Unsupported baud rate") | ||
| args = [0x03, 0x01, (divider >> 8) & 0xFF, divider & 0xFF] | ||
| self._run_command(_SET_PORT, bytes(args), 7) | ||
| self._uart.baudrate = baud |
There was a problem hiding this comment.
| def set_baudrate(self, baud): | |
| """As above. For internal use.""" | |
| divider = None | |
| if baud == 9600: | |
| divider = _BAUDRATE_9600 | |
| elif baud == 19200: | |
| divider = _BAUDRATE_19200 | |
| elif baud == 38400: | |
| divider = _BAUDRATE_38400 | |
| elif baud == 57600: | |
| divider = _BAUDRATE_57600 | |
| elif baud == 115200: | |
| divider = _BAUDRATE_115200 | |
| else: | |
| raise ValueError("Unsupported baud rate") | |
| args = [0x03, 0x01, (divider >> 8) & 0xFF, divider & 0xFF] | |
| self._run_command(_SET_PORT, bytes(args), 7) | |
| self._uart.baudrate = baud |
| def set_img_size(self, size): | ||
| """Literally the above, but for use internally.""" | ||
| if size not in (IMAGE_SIZE_640x480, IMAGE_SIZE_320x240, IMAGE_SIZE_160x120): | ||
| raise ValueError( | ||
| "Size must be one of IMAGE_SIZE_640x480, IMAGE_SIZE_320x240, or " | ||
| "IMAGE_SIZE_160x120!" | ||
| ) | ||
| return self._run_command( | ||
| _WRITE_DATA, bytes([0x05, 0x04, 0x01, 0x00, 0x19, size & 0xFF]), 5 | ||
| ) | ||
| | ||
| def get_image_size(self): | ||
| """Get the current image size. Will return a WIDTH, HEIGHT tuple.""" | ||
| size = self.image_size | ||
| if size == 0x00: | ||
| size = (640, 480) | ||
| elif size == 0x11: | ||
| size = (320, 240) | ||
| elif size == 0x22: | ||
| size = (160, 120) | ||
| return size | ||
| |
There was a problem hiding this comment.
| def set_img_size(self, size): | |
| """Literally the above, but for use internally.""" | |
| if size not in (IMAGE_SIZE_640x480, IMAGE_SIZE_320x240, IMAGE_SIZE_160x120): | |
| raise ValueError( | |
| "Size must be one of IMAGE_SIZE_640x480, IMAGE_SIZE_320x240, or " | |
| "IMAGE_SIZE_160x120!" | |
| ) | |
| return self._run_command( | |
| _WRITE_DATA, bytes([0x05, 0x04, 0x01, 0x00, 0x19, size & 0xFF]), 5 | |
| ) | |
| def get_image_size(self): | |
| """Get the current image size. Will return a WIDTH, HEIGHT tuple.""" | |
| size = self.image_size | |
| if size == 0x00: | |
| size = (640, 480) | |
| elif size == 0x11: | |
| size = (320, 240) | |
| elif size == 0x22: | |
| size = (160, 120) | |
| return size |
| # Get the image size const | ||
| size = get_image_size(width, height) | ||
| # And set it. | ||
| self.set_img_size(size) |
There was a problem hiding this comment.
| self.set_img_size(size) | |
| self.img_size = size |
| self.resume_video() | ||
| return True | ||
| | ||
| def stop_video(self): |
There was a problem hiding this comment.
| def stop_video(self): | |
| def _stop_video(self): |
| raise RuntimeError("Failed to halt frame!") | ||
| return attempt | ||
| | ||
| def resume_video(self): |
There was a problem hiding this comment.
| def resume_video(self): | |
| def _resume_video(self): |
| """ | ||
| return self._run_command(_FBUF_CTRL, bytes([0x1, _RESUMEFRAME]), 5) | ||
| | ||
| def read_picture_into(self, buf): |
There was a problem hiding this comment.
| def read_picture_into(self, buf): | |
| def _read_picture_into(self, buf): |
| @Andon-A Hey! Thanks for the contribution. Scott suggested a few changes. Are you still interested in working on this? |
| @kattni Oh gosh, I completely forgot about this. I'll try to find some time to look at it in the upcoming few weeks and remember where I was. |
| I saw that there's camera support being worked on for CircuitPython. I have no idea when I'll end up working on this again, and as the camera updates may change how things are wanted, I'm just going to close this and come back to it at a later date. |
| @jepler Food for thought. |
3 participants
Adds a function called save_image to allow for, unsurprisingly, saving the image using only the library. Then added a function called take_and_save that will take the image, save it, and then turn the camera back to video mode. Both examples are updated to take advantage of this.
take_and_save takes three arguments: Filename (Surprising, I know), overwrite, and image size. While I've left a lot of explanations and things, I've taken a picture with four lines of code:
Nothing in the library has been re-named or even modified, only added to, so existing code shouldn't be affected. I've tested with no issues on CircuitPython and Windows via a FTDI connector.