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

Updated Micro epsilon SDK and dependencies #32

Merged
merged 6 commits into from
Nov 24, 2023

Conversation

NithishkumarS
Copy link
Contributor

@NithishkumarS NithishkumarS commented Nov 22, 2023

Update the driver and Dockerfile used for build testing to v1.0.0 of the SDK (currently the latest version) for the Micro Epsilon scanCONTROL devices.

Jira issue: LH2-210

Motivation and Context

The current SDK version (v0.2.5) is outdated and does not support the latest devices.

Changes

  • Update the Dockerfile to use the latest SDK version
    • Updated scanCONTROL SDK to v1.0.0
    • Updated Aravis to 0.8.30

Type of changes

  • Breaking change (Updated the docker image to use Ubuntu focal and ros noetic)

Checklist

  • Update version
  • Update dependencies
  • Test Docker build

.docker/Dockerfile Outdated Show resolved Hide resolved
.docker/Dockerfile Outdated Show resolved Hide resolved
.docker/Dockerfile Outdated Show resolved Hide resolved
.docker/Dockerfile Outdated Show resolved Hide resolved
.docker/Dockerfile Outdated Show resolved Hide resolved
.docker/Dockerfile Outdated Show resolved Hide resolved
@dave992 dave992 force-pushed the LH2-210-Update-SDK-version-micro-epsilon branch from 181cf91 to 8bcd331 Compare November 23, 2023 15:59
meson setup build && \
cd build && \
ninja && \
checkinstall --pkgname aravis --pkgversion 0.8.30 --requires="libglib2.0-dev, libxml2-dev" ninja install
Copy link
Member

Choose a reason for hiding this comment

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

just curious why we 'bother' with checkinstall if this is a Dockerfile?

@NithishkumarS NithishkumarS merged commit c91a7e5 into master Nov 24, 2023
2 checks passed
pkg-config \
unzip \
wget \
&& rm -rf /var/lib/apt/lists/*
xsltproc \
&& rm -rf /var/lib/apt/lists/* && \
Copy link

@katettudelft katettudelft Nov 28, 2023

Choose a reason for hiding this comment

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

I'd suggest expanding this a bit further - this is the standard 'clean-up' stanza I use with each apt call:

apt-get autoremove -y &&\
apt-get clean -y &&\
rm -rf /var/lib/apt/lists/*


FROM ros:melodic-ros-core
FROM ros:noetic-ros-core

Choose a reason for hiding this comment

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

I'd recommend that you add an AS label for FROM statements. It breaks the file up a bit conceptually, and makes multi-target builds an option (if we want to use them later).

There's an example on line 3.

intltool \
pkg-config \
&& rm -rf /var/lib/apt/lists/*

COPY --from=build ["/library_pkgs", "/library_pkgs"]

RUN apt-get update && \
apt install -y /library_pkgs/aravis_0.6.4-1_amd64.deb && \
apt install -y /library_pkgs/aravis_0.8.30-1_amd64.deb && \

Choose a reason for hiding this comment

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

It's preferable to use apt-get rather than apt in scripts - it has a more reliable command line interface.

I think you can do all of the packages in one install command too (but please test it works if you change it).

Finally, if these are the only files in /library_pkgs/, then you could consider using
apt-get install /library_pkgs/* to make the command shorter. Obviously if there are additional files in there that you don't want, or the contents are subject to change, then don't do this. :)

Pugens added a commit to Pugens/scancontrol that referenced this pull request Feb 15, 2024
…icro-epsilon

Updated Micro epsilon SDK and dependencies

(cherry picked from commit c91a7e5)
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.

4 participants