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

RSDK-4369 - Create jammy base and rebase cpp-sdk #9

Merged
merged 15 commits into from
Nov 8, 2023

Conversation

seanavery
Copy link
Collaborator

Description

  • Creates new jammy base image for building the appimage.
  • Adjusts module for latest cpp-sdk and newer protobuf version.
  • Adds documentation for Raspberry Pi usage.

@@ -46,11 +46,11 @@ void CSICamera::set_attr(const AttributeMap& attrs, const std::string& name, T C
auto val = proto->proto_value();

if constexpr (std::is_same<T, int>::value) {
this->*member = val.has_number_value() ? val.number_value() : de;
this->*member = val.number_value();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Newer protobuf version no longer includes has_X_value member.

@@ -66,7 +66,7 @@ void CSICamera::reconfigure(const Dependencies deps, const ResourceConfig cfg) {
init(attrs);
}

Camera::raw_image CSICamera::get_image(const std::string mime_type) {
Camera::raw_image CSICamera::get_image(const std::string mime_type, const AttributeMap& extra) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Camera API in cpp-sdk changed here.

@@ -20,10 +20,10 @@ AppDir:
files:
include:
- /usr/local/lib/libviamsdk.so.noabi
- /lib/aarch64-linux-gnu/libpthread*
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had to add pthread include explicitly in bundle when moving from focal to jammy.

@seanavery seanavery marked this pull request as draft October 26, 2023 20:16
@seanavery seanavery marked this pull request as ready for review October 30, 2023 18:37
Copy link
Contributor

@bazile-clyde bazile-clyde left a comment

Choose a reason for hiding this comment

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

Only one comment but otherwise, LGTM! The docs clean up make things a lot more clear btw! Great job!

README.md Outdated
```bash
sudo wget http://packages.viam.com/apps/csi-camera/viam-csi-latest-aarch64.AppImage -O /usr/local/bin/viam-csi
sudo apt install libgstreamer-plugins-base1.0-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

If users only need to install base1 why does our makefile install good/bad1? We should probably make our setups and the docs users will use consistent, so we can catch any errors they may have. Here and in PI.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed gstreamer dependency instructions here to include base, good and bad plugin sets.

Copy link
Collaborator Author

@seanavery seanavery left a comment

Choose a reason for hiding this comment

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

This merge works on Jetson and Pi Bullseye devices. On Bookworm, there is an issue linking the new libcamera gst plugin which includes trace ltt embeds that deadlock when running inside of an appimage.


_Note: On a Raspberry Pi, you must install GStreamer plugins before running the module._

_WARNING: There is a known issue for Debian Bookworm due to changes in the libcamerasrc plugin._
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This merge does not include a bundle that works on Bookworm.

@seanavery seanavery merged commit c371ac3 into viam-modules:main Nov 8, 2023
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.

2 participants