Skip to content

2D API wrapper - #28

Merged
trym-b merged 1 commit into
masterfrom
python-2d-api
Nov 26, 2019
Merged

2D API wrapper#28
trym-b merged 1 commit into
masterfrom
python-2d-api

Conversation

@trym-b

@trym-btrym-b commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

Enables 2D API in this wrapper, so that you can easily capture 2D images
using a camera. 2D sample capture and tests has been included.

@trym-b
trym-b requested a review from nedreboNovember 13, 2019 05:37
@eskaur
eskaur self-requested a review November 18, 2019 11:29
eskaur
eskaur reviewed Nov 18, 2019
View reviewed changes

@eskaureskaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks very good to me. Great test-coverage too.
I just have a few minor comments/questions.

Comment threadsrc/ReleasableImage.cpp Outdated
Comment threadmodules/zivid/frame_2d.py Outdated
Comment threadmodules/zivid/camera.py
Comment threadsrc/ReleasableImage.cpp Outdated
Comment threadsrc/Wrapper.cpp Outdated
Comment threadsrc/include/ZividPython/Wrappers.h Outdated
Comment threadtest/test_frame_2d.py Outdated
@trym-b

trym-b commented Nov 19, 2019

Copy link
Copy Markdown
ContributorAuthor

Ready for next round

nedrebo
nedrebo reviewed Nov 19, 2019
View reviewed changes
Comment threadsrc/ReleasableImage.cpp Outdated
nedrebo
nedrebo reviewed Nov 19, 2019
View reviewed changes
Comment threadsrc/Wrapper.cpp Outdated
eskaur
eskaur previously approved these changes Nov 19, 2019
View reviewed changes

@eskaureskaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me now.

@trym-b

trym-b commented Nov 20, 2019

Copy link
Copy Markdown
ContributorAuthor

Ready for next round @nedrebo

nedrebo
nedrebo reviewed Nov 21, 2019
View reviewed changes

@nedrebonedrebo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good, only minor comments.

Comment threadmodules/zivid/__init__.py Outdated
Comment threadmodules/zivid/__init__.py Outdated
Comment threadmodules/zivid/image.py
Comment threadsrc/include/ZividPython/Releasable.h Outdated
@trym-b
trym-bforce-pushed the python-2d-api branch 2 times, most recently from 2f28ac1 to 62a10a1CompareNovember 25, 2019 05:44
nedrebo
nedrebo reviewed Nov 25, 2019
View reviewed changes
Comment threadsrc/ReleasableImage.cpp Outdated

return py::buffer_info{
const_cast<Zivid::RGBA8 *>(
data), // TODO: Const casting this until https://github.com/pybind/pybind11/issues/1993 has been solved

@nedrebonedreboNov 25, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is now merged. The fix is now to simply remove the const cast once pybind11 is updated.

@trym-btrym-bNov 25, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Updated the comment

@trym-b
trym-bforce-pushed the python-2d-api branch 3 times, most recently from 5405790 to 9339676CompareNovember 25, 2019 10:32
apartridge
apartridge reviewed Nov 25, 2019
View reviewed changes
Comment threadmodules/zivid/frame_2d.py
@trym-b

trym-b commented Nov 26, 2019•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Any more comments @apartridge, @nedrebo or @eskaur ?

eskaur
eskaur previously approved these changes Nov 26, 2019
View reviewed changes

@eskaureskaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

Trym Bremnes
2D API wrapper
14312c5
Enables 2D API in this wrapper, so that you can easily capture 2D images
using a camera. 2D sample capture and tests has been included.
@trym-b
trym-b merged commit c4bb9e8 into masterNov 26, 2019
@trym-b
trym-b deleted the python-2d-api branch November 26, 2019 09:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@apartridgeapartridgeapartridge left review comments
@nedrebonedrebonedrebo left review comments
@eskaureskaureskaur approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@trym-b@apartridge@nedrebo@eskaur