Skip to content
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

Add support for TIS Camera #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add support for TIS Camera #67

wants to merge 2 commits into from

Conversation

kwang428
Copy link

@kwang428 kwang428 commented Jan 3, 2024

Regarding issue #25 , here is a commit that integrates an Imaging Source Camera. One mysterious hard-coded factor in the code is a factor of 3 for RGB color. Even on monochromatic formats, the tisgrabber code still appears to return RGB color, where each component is the same. To obtain the image, this code just grabs the "R" component.

Otherwise, I welcome any suggestions or changes!

nacslab and others added 2 commits January 3, 2024 12:19
Hi @kwang428 ,

First, I wanted to apologize for taking so long to get to this pull request. This is great work and we really appreciate your contributions.

I made some small suggested edits to style to conform with PEP and the rest of the package. I changed the name of the file and the class to likewise conform.

I also removed the `width` and `height` variables because the superclass encodes them in the `shape = (height, width)` attribute.

I think the only changes to behavior that I did are:
- remove a `/ 8` in the bitdepth variable. 24 = 3 * 8 implies 8-bit depth.
- fix a comma bug in the else case of the woi function.

Apart from some lingering concerns about the forced-RGB nature of the camera, it looks good.

At this point, I am alright with pushing to main, but if you have time to test the current code on hardware, that would be preferred to pushing slightly untested code.

Let me know what you think!

Best,
Ian
Copy link
Collaborator

@ichristen ichristen left a comment

Choose a reason for hiding this comment

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

Hi @kwang428 ,

Please see my commit message in 114b652. Let me know what you think!

Best,
Ian

@ichristen ichristen mentioned this pull request Feb 20, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants