From 7ff05a7e08e3b0151ffc43b87dda5ce8c338c0bc Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 23 Sep 2024 15:41:49 -0400 Subject: [PATCH 1/7] first attempt conan ci job --- .github/workflows/conan.yml | 165 ++++++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 .github/workflows/conan.yml diff --git a/.github/workflows/conan.yml b/.github/workflows/conan.yml new file mode 100644 index 000000000..1071e3f9d --- /dev/null +++ b/.github/workflows/conan.yml @@ -0,0 +1,165 @@ +name: Test conan packages + +on: + workflow_dispatch: + +jobs: + build_macos: + if: github.repository_owner == 'viamrobotics' + runs-on: macos-latest + strategy: + fail-fast: false + matrix: + include: + - target: aarch64-apple-darwin + platform: macosx_arm64 + - target: x86_64-apple-darwin + platform: macosx_x86_64 + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + brew install cmake + brew install python ninja buf + python -m pip install --user conan + + - name: Build + run: | + # `buf` tries to read a CLI config file that we don't actually use located at + # ~/.config/buf/config.yaml. We don't always have permission to access this + # directory in CI, so we set the `BUF_CONFIG_DIR` to some other value that we + # do have permissions for. See https://github.com/bufbuild/buf/issues/2698 for + # more details. + export BUF_CONFIG_DIR=$(mktemp -d) + conan profile detect + conan create . + + build_linux_ubuntu_jammy: + if: github.repository_owner == 'viamrobotics' + runs-on: ${{ matrix.runs_on }} + container: + image: ${{ matrix.image }} + strategy: + fail-fast: false + matrix: + include: + - target: aarch64-ubuntu-jammy-gnu + platform: linux_aarch64-ubuntu-jammy + image: ubuntu:22.04 + runs_on: buildjet-8vcpu-ubuntu-2204-arm + - target: x86_64-ubuntu-jammy-gnu + platform: linux_x86_64-ubuntu-jammy + image: ubuntu:22.04 + runs_on: buildjet-8vcpu-ubuntu-2204 + + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + apt-get update + apt-get -y dist-upgrade + DEBIAN_FRONTEND=noninteractive apt-get -y --no-install-recommends install \ + build-essential \ + ca-certificates \ + curl \ + doxygen \ + g++ \ + gdb \ + git \ + gnupg \ + gpg \ + less \ + ninja-build \ + python3 \ + python3-pip \ + software-properties-common \ + sudo \ + wget \ + + sudo wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/null | sudo gpg --dearmor - | sudo tee /usr/share/keyrings/kitware-archive-keyring.gpg >/dev/null + sudo echo 'deb [signed-by=/usr/share/keyrings/kitware-archive-keyring.gpg] https://apt.kitware.com/ubuntu/ jammy main' | sudo tee /etc/apt/sources.list.d/kitware.list >/dev/null + + apt-get update + apt-get -y install cmake + + pip install conan + + - name: Create package + shell: bash + run: | + conan profile detect + conan create . + + build_linux_debian: + if: github.repository_owner == 'viamrobotics' + runs-on: ${{ matrix.runs_on }} + container: + image: ${{ matrix.image }} + strategy: + fail-fast: false + matrix: + include: + - target: aarch64-debian-bullseye + platform: linux_aarch64-debian-bullseye + image: debian:bullseye + runs_on: buildjet-8vcpu-ubuntu-2204-arm + - target: x86_64-debian-bullseye + platform: linux_x86_64-debian-bullseye + image: debian:bullseye + runs_on: buildjet-8vcpu-ubuntu-2204 + - target: aarch64-debian-bookworm + platform: linux_aarch64-debian-bookworm + image: debian:bookworm + runs_on: buildjet-8vcpu-ubuntu-2204-arm + - target: x86_64-debian-bookworm + platform: linux_x86_64-debian-bookworm + image: debian:bookworm + runs_on: buildjet-8vcpu-ubuntu-2204 + + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + apt-get update + apt-get -y dist-upgrade + DEBIAN_FRONTEND=noninteractive apt-get -y --no-install-recommends install \ + build-essential \ + ca-certificates \ + cmake \ + curl \ + g++ \ + gdb \ + git \ + gnupg \ + gpg \ + less \ + ninja-build \ + python3 \ + python3-pip \ + software-properties-common \ + sudo \ + wget + + pip install conan + + - name: Update CMake for bullseye + if: ${{ matrix.image == 'debian:bullseye' }} + run: | + apt-add-repository -y 'deb http://deb.debian.org/debian bullseye-backports main' + + apt-get update + apt-get -y install cmake + + apt-get -y --no-install-recommends install -t bullseye-backports cmake + + - name: Create package + shell: bash + run: | + conan profile detect + conan create . From b04fc36d60a17c910223a62bc191ebf3aaf8cdd0 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 23 Sep 2024 15:42:14 -0400 Subject: [PATCH 2/7] remove git --- .github/workflows/conan.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/conan.yml b/.github/workflows/conan.yml index 1071e3f9d..d4d03f870 100644 --- a/.github/workflows/conan.yml +++ b/.github/workflows/conan.yml @@ -69,7 +69,6 @@ jobs: doxygen \ g++ \ gdb \ - git \ gnupg \ gpg \ less \ @@ -135,7 +134,6 @@ jobs: curl \ g++ \ gdb \ - git \ gnupg \ gpg \ less \ From 95f407e289a2705cd923fa8aed60415423ee1179 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:53:46 -0400 Subject: [PATCH 3/7] comment out repo owner --- .github/workflows/conan.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/conan.yml b/.github/workflows/conan.yml index d4d03f870..d4278b0b6 100644 --- a/.github/workflows/conan.yml +++ b/.github/workflows/conan.yml @@ -5,7 +5,7 @@ on: jobs: build_macos: - if: github.repository_owner == 'viamrobotics' + # if: github.repository_owner == 'viamrobotics' runs-on: macos-latest strategy: fail-fast: false @@ -37,7 +37,7 @@ jobs: conan create . build_linux_ubuntu_jammy: - if: github.repository_owner == 'viamrobotics' + # if: github.repository_owner == 'viamrobotics' runs-on: ${{ matrix.runs_on }} container: image: ${{ matrix.image }} @@ -94,7 +94,7 @@ jobs: conan create . build_linux_debian: - if: github.repository_owner == 'viamrobotics' + # if: github.repository_owner == 'viamrobotics' runs-on: ${{ matrix.runs_on }} container: image: ${{ matrix.image }} From c5bc9a6d386d8720deca78c9fce1cbae292d39d9 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 24 Sep 2024 15:00:50 -0400 Subject: [PATCH 4/7] global install --- .github/workflows/conan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/conan.yml b/.github/workflows/conan.yml index d4278b0b6..3634a4869 100644 --- a/.github/workflows/conan.yml +++ b/.github/workflows/conan.yml @@ -23,7 +23,7 @@ jobs: run: | brew install cmake brew install python ninja buf - python -m pip install --user conan + python -m pip install conan - name: Build run: | From be04812206a50a7209cdacd867387e71fef5011b Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 24 Sep 2024 15:09:15 -0400 Subject: [PATCH 5/7] always set build = missing --- .github/workflows/conan.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/conan.yml b/.github/workflows/conan.yml index 3634a4869..52ca377de 100644 --- a/.github/workflows/conan.yml +++ b/.github/workflows/conan.yml @@ -34,7 +34,7 @@ jobs: # more details. export BUF_CONFIG_DIR=$(mktemp -d) conan profile detect - conan create . + conan create . --build=missing build_linux_ubuntu_jammy: # if: github.repository_owner == 'viamrobotics' @@ -91,7 +91,7 @@ jobs: shell: bash run: | conan profile detect - conan create . + conan create . --build=missing build_linux_debian: # if: github.repository_owner == 'viamrobotics' @@ -160,4 +160,4 @@ jobs: shell: bash run: | conan profile detect - conan create . + conan create . --build=missing From 6215c4b5195cfa2ceb36ec238cd14da89d84745d Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 4 Oct 2024 17:14:28 -0400 Subject: [PATCH 6/7] use get api --- .../examples/modules/complex/gizmo/impl.cpp | 15 ++-- .../modules/complex/summation/impl.cpp | 7 +- src/viam/examples/modules/simple/main.cpp | 18 +++-- src/viam/examples/modules/tflite/main.cpp | 68 ++++++++----------- 4 files changed, 49 insertions(+), 59 deletions(-) diff --git a/src/viam/examples/modules/complex/gizmo/impl.cpp b/src/viam/examples/modules/complex/gizmo/impl.cpp index 7a9272d74..adbf8675a 100644 --- a/src/viam/examples/modules/complex/gizmo/impl.cpp +++ b/src/viam/examples/modules/complex/gizmo/impl.cpp @@ -21,15 +21,14 @@ std::string find_arg1(ResourceConfig cfg) { buffer << gizmo_name << ": Required parameter `arg1` not found in configuration"; throw std::invalid_argument(buffer.str()); } - - const ProtoValue& arg1_val = arg1->second; - if (arg1_val.is_a() && !arg1_val.get_unchecked().empty()) { - return arg1_val.get_unchecked(); + const auto* const arg1_string = arg1->second.get(); + if (!arg1_string || arg1_string->empty()) { + std::ostringstream buffer; + buffer << gizmo_name << ": Required non-empty string parameter `arg1`" + << "` is either not a string or is an empty string"; + throw std::invalid_argument(buffer.str()); } - std::ostringstream buffer; - buffer << gizmo_name << ": Required non-empty string parameter `arg1`" - << "` is either not a string or is an empty string"; - throw std::invalid_argument(buffer.str()); + return *arg1_string; } void MyGizmo::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) { diff --git a/src/viam/examples/modules/complex/summation/impl.cpp b/src/viam/examples/modules/complex/summation/impl.cpp index 05e499e29..03231e38f 100644 --- a/src/viam/examples/modules/complex/summation/impl.cpp +++ b/src/viam/examples/modules/complex/summation/impl.cpp @@ -17,8 +17,11 @@ bool find_subtract(ResourceConfig cfg) { if (subtract == cfg.attributes().end()) { return false; } - - return subtract->second.is_a() && subtract->second.get_unchecked(); + const bool* const subtract_bool = subtract->second.get(); + if (!subtract_bool) { + return false; + } + return *subtract_bool; } void MySummation::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) { diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp index 4e506d89b..edc400347 100644 --- a/src/viam/examples/modules/simple/main.cpp +++ b/src/viam/examples/modules/simple/main.cpp @@ -59,17 +59,15 @@ class Printer : public GenericService, public Reconfigurable { buffer << printer_name << ": Required parameter `to_print` not found in configuration"; throw std::invalid_argument(buffer.str()); } - const ProtoValue& to_print_val = to_print->second; - if (to_print_val.is_a() && - !to_print_val.get_unchecked().empty()) { - return to_print_val.get_unchecked(); + const auto* const to_print_string = to_print->second.get(); + if (!to_print_string || to_print_string->empty()) { + std::ostringstream buffer; + buffer << printer_name + << ": Required non-empty string parameter `to_print` is either not a string " + "or is an empty string"; + throw std::invalid_argument(buffer.str()); } - - std::ostringstream buffer; - buffer << printer_name - << ": Required non-empty string parameter `to_print` is either not a string " - "or is an empty string"; - throw std::invalid_argument(buffer.str()); + return *to_print_string; } private: diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index 3dd17621f..b700353f6 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -298,59 +298,55 @@ class MLModelServiceTFLite : public vsdk::MLModelService, << ": Required parameter `model_path` not found in configuration"; throw std::invalid_argument(buffer.str()); } - - const vsdk::ProtoValue& model_path_val = model_path->second; - if (!model_path_val.is_a() || - model_path_val.get_unchecked().empty()) { + const auto* const model_path_string = model_path->second.get(); + if (!model_path_string || model_path_string->empty()) { std::ostringstream buffer; buffer << service_name << ": Required non-empty string parameter `model_path` is either not a string " "or is an empty string"; throw std::invalid_argument(buffer.str()); } - const std::string& model_path_string = model_path_val.get_unchecked(); // Process any tensor name remappings provided in the config. auto remappings = attributes.find("tensor_name_remappings"); if (remappings != attributes.end()) { - if (!remappings->second.is_a()) { + const auto remappings_attributes = remappings->second.get(); + if (!remappings_attributes) { std::ostringstream buffer; buffer << service_name << ": Optional parameter `tensor_name_remappings` must be a dictionary"; throw std::invalid_argument(buffer.str()); } - const auto remappings_attributes = - remappings->second.get_unchecked(); - const auto populate_remappings = [](const vsdk::ProtoValue& source, auto& target) { - if (!source.is_a()) { + const auto source_attributes = source.get(); + if (!source_attributes) { std::ostringstream buffer; buffer << service_name - << ": Fields `inputs` and `outputs` of `tensor_name_remappings` " - "must be " + << ": Fields `inputs` and `outputs` of `tensor_name_remappings` must be " "dictionaries"; throw std::invalid_argument(buffer.str()); } - for (const auto& kv : source.get_unchecked()) { + for (const auto& kv : *source_attributes) { const auto& k = kv.first; - if (!kv.second.is_a()) { + const auto* const kv_string = kv.second.get(); + if (!kv_string) { std::ostringstream buffer; - buffer << service_name - << ": Fields `inputs` and `outputs` of `tensor_name_remappings` " - "must " - "be dictionaries with string values"; + buffer + << service_name + << ": Fields `inputs` and `outputs` of `tensor_name_remappings` must " + "be dictionaries with string values"; throw std::invalid_argument(buffer.str()); } - target[kv.first] = kv.second.get_unchecked(); + target[kv.first] = *kv_string; } }; - const auto inputs_where = remappings_attributes.find("inputs"); - if (inputs_where != remappings_attributes.end()) { + const auto inputs_where = remappings_attributes->find("inputs"); + if (inputs_where != remappings_attributes->end()) { populate_remappings(inputs_where->second, state->input_name_remappings); } - const auto outputs_where = remappings_attributes.find("outputs"); - if (outputs_where != remappings_attributes.end()) { + const auto outputs_where = remappings_attributes->find("outputs"); + if (outputs_where != remappings_attributes->end()) { populate_remappings(outputs_where->second, state->output_name_remappings); } } @@ -366,11 +362,11 @@ class MLModelServiceTFLite : public vsdk::MLModelService, // buffer which we can use with `TfLiteModelCreate`. That // still requires that the buffer be kept valid, but that's // more easily done. - const std::ifstream in(model_path_string, std::ios::in | std::ios::binary); + const std::ifstream in(*model_path_string, std::ios::in | std::ios::binary); if (!in) { std::ostringstream buffer; buffer << service_name << ": Failed to open file for `model_path` " - << model_path_string; + << *model_path_string; throw std::invalid_argument(buffer.str()); } std::ostringstream model_path_contents_stream; @@ -405,27 +401,21 @@ class MLModelServiceTFLite : public vsdk::MLModelService, // object to carry that information. auto num_threads = attributes.find("num_threads"); if (num_threads != attributes.end()) { - auto throwError = [&] { + const auto* num_threads_double = num_threads->second.get(); + if (!num_threads_double || !std::isnormal(*num_threads_double) || + (*num_threads_double < 0) || + (*num_threads_double >= std::numeric_limits::max()) || + (std::trunc(*num_threads_double) != *num_threads_double)) { std::ostringstream buffer; buffer << service_name - << ": Value for field `num_threads` is not a positive integer"; + << ": Value for field `num_threads` is not a positive integer: " + << *num_threads_double; throw std::invalid_argument(buffer.str()); - }; - - if (!num_threads->second.is_a()) { - throwError(); - } - - double num_threads_double = num_threads->second.get_unchecked(); - if (!std::isnormal(num_threads_double) || (num_threads_double < 0) || - (num_threads_double >= std::numeric_limits::max()) || - (std::trunc(num_threads_double) != num_threads_double)) { - throwError(); } state->interpreter_options.reset(TfLiteInterpreterOptionsCreate()); TfLiteInterpreterOptionsSetNumThreads(state->interpreter_options.get(), - static_cast(num_threads_double)); + static_cast(*num_threads_double)); } // Build the single interpreter. From e79b8672df8ab5b16a44c4fdd3dbd6c3cbd40f73 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 4 Oct 2024 17:21:11 -0400 Subject: [PATCH 7/7] this was actually cleaner --- src/viam/examples/modules/complex/summation/impl.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/viam/examples/modules/complex/summation/impl.cpp b/src/viam/examples/modules/complex/summation/impl.cpp index 03231e38f..05e499e29 100644 --- a/src/viam/examples/modules/complex/summation/impl.cpp +++ b/src/viam/examples/modules/complex/summation/impl.cpp @@ -17,11 +17,8 @@ bool find_subtract(ResourceConfig cfg) { if (subtract == cfg.attributes().end()) { return false; } - const bool* const subtract_bool = subtract->second.get(); - if (!subtract_bool) { - return false; - } - return *subtract_bool; + + return subtract->second.is_a() && subtract->second.get_unchecked(); } void MySummation::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {