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.
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
DOCS-923: Document
viam:camera:csi
modular camera as model for the Jetson Orin Nano's embedded camera #1716DOCS-923: Document
viam:camera:csi
modular camera as model for the Jetson Orin Nano's embedded camera #1716Changes from 4 commits
3f8bc47
405e73b
c5f6c8e
9490f17
fc1fcea
ff5d2d1
e984fb5
7886cea
707db27
640e855
22ce75d
3002dda
7899941
64c4610
a432a3f
f448729
995926d
0422070
1dbfe66
2901c6a
1c20497
b930ad0
8ab21a9
81f10bb
6d809ef
813961d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thin kyou want to provide the value before the explanation - what does this resource allow readers to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok done I think. Sentence is a little bare though-- maybe revisit this feedback tmrw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I will revisit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more talking about what I think is the value-add of CSI cameras and explaining more about what they are-- lmk what you think. Using info from here but not sure whether or not to link @npentrel https://embeddedcomputing.com/technology/analog-and-power/mipi-csi-2-v3-0-enhances-imaging-reduces-cabling-for-embedded-systems-designers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good but I think what's missing is why I'd need to use this over the normal camera component - see suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds good. In the context of Jetson devices one of the major advantages is using the hardware-accelerated ISP (image signal processing) library on the host machine called libargus.
Also, mechanically reliable connection from the camera to the host machine on mobile robotics platforms. (As opposed to using usb cables/ports).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing article in
Install requirements
but that also doesn't sound super great? Should that be "get the modular resouce" or something like that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm I can change here but I don't know if we actually need to change the beginning of the sentence! I think that without article is actually the normal way to say it, but I could be wrong 😆 Requirements as a noun and directive, right? Like a medium article?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will want to update the install instructions once it is uploaded to the module registry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the attribute parameters!