Skip to content

Conversation

dmitry1945
Copy link
Collaborator

Checklist

  • [x ] Component contains License
  • [ x] Component contains README.md
  • [ x] Component contains idf_component.yml file with url field defined
  • [ x] Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Change description

Add an Inverse Kinematic library as a component into the component list.
The component contain simple example.

@suda-morris suda-morris requested a review from Copilot July 3, 2025 15:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates the IK (inverse kinematics) library as a new ESP-IDF component, setting up its metadata, build integration, and a usage example, and updates CI/workflows accordingly.

  • Added SBOM and IDF component manifests, LICENSE, and README for iklib
  • Configured CMake to fetch, patch, build, and link the ik submodule via ExternalProject
  • Provided a “hello-ik” example and updated Git submodules plus upload/build CI rules

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
iklib/sbom_iklib.yml SBOM metadata for iklib
iklib/patch/ik_patch.patch Modified target_compile_options flags
iklib/README.md Added component README (needs title/badge corrections)
iklib/CMakeLists.txt Setup ExternalProject build, patch application
iklib/examples/hello-ik/main/* Example app manifest, source, and CMakeLists
.gitmodules Added iklib/ik submodule
.github/workflows/upload_component.yml Included iklib in upload pipeline
.idf_build_apps.toml Enabled example build test for iklib
Comments suppressed due to low confidence (3)

iklib/README.md:1

  • The title references JPEG-Turbo—update it to reflect the IK library (e.g., # IK for ESP-IDF).
# Jpeg-turbo for ESP-IDF

iklib/README.md:3

  • This badge points to libjpeg-turbo; replace it with the correct badge and URL for iklib.
[![Component Registry](https://components.espressif.com/components/espressif/libjpeg-turbo/badge.svg)](https://components.espressif.com/components/espressif/libjpeg-turbo)

iklib/CMakeLists.txt:99

  • The variable deps is never defined, so no dependencies will be linked. Consider using COMPONENT_REQUIRES or define deps appropriately.
foreach(dep ${deps})

version: 1.1.0
cpe: cpe:1.1.0:a:iklib:iklib:{}:*:*:*:*:*:*:*
supplier: 'Organization: iklib'
description: IK is a inverse kinematic library
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Grammar nit: replace "a inverse" with "an inverse" for correct article usage.

Suggested change
description: IK is a inverse kinematic library
description: IK is an inverse kinematic library

Copilot uses AI. Check for mistakes.

Comment on lines +10 to +19
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
>
PRIVATE $<$<C_COMPILER_ID:Clang>:
-W -Wall -Wextra -Werror -Wshadow -Wconversion -Wno-unused-parameter -Wno-conversion -Wno-implicit-fallthrough
- -pedantic -pedantic-errors -fno-strict-aliasing -ffast-math
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Profiling mode no longer includes -pg, which disables instrumentation; reintroduce -pg inside the generator expression, e.g., $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>.

Suggested change
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
>
PRIVATE $<$<C_COMPILER_ID:Clang>:
-W -Wall -Wextra -Werror -Wshadow -Wconversion -Wno-unused-parameter -Wno-conversion -Wno-implicit-fallthrough
- -pedantic -pedantic-errors -fno-strict-aliasing -ffast-math
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
>
PRIVATE $<$<C_COMPILER_ID:Clang>:
-W -Wall -Wextra -Werror -Wshadow -Wconversion -Wno-unused-parameter -Wno-conversion -Wno-implicit-fallthrough
- -pedantic -pedantic-errors -fno-strict-aliasing -ffast-math
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>

Copilot uses AI. Check for mistakes.

- -pedantic -pedantic-errors -fno-strict-aliasing -ffast-math
- $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>
+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Same as above for Clang: profiling flags omit -pg. Add it back in the generator expression to enable profiling instrumentation.

Suggested change
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
+ $<$<BOOL:IK_PROFILING>:-pg -fno-omit-frame-pointer>

Copilot uses AI. Check for mistakes.

iklib/README.md Outdated
@@ -0,0 +1,12 @@
# Jpeg-turbo for ESP-IDF
Copy link
Collaborator

Choose a reason for hiding this comment

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

jpeg-turbo ==> ik

+ -fno-strict-aliasing -ffast-math
+ $<$<BOOL:IK_PROFILING>: -fno-omit-frame-pointer>
>
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like this library is not maintained anymore in the upstream? TheComet/ik#22 (comment)

Should we fork it and patch it in our fork? cc @igrr

BTW, this repo also provide a promising IK library: https://github.com/roboticslibrary/rl/blob/master/src/rl/mdl/InverseKinematics.h

@suda-morris suda-morris requested a review from igrr July 3, 2025 15:31
@@ -0,0 +1,12 @@
# IK (Inverse Kinematic) library for ESP-IDF

[![Component Registry](https://components.espressif.com/components/espressif/libjpeg-turbo/badge.svg)](https://components.espressif.com/components/espressif/libjpeg-turbo)
Copy link
Collaborator

Choose a reason for hiding this comment

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

jpeg->iklib

#include <stdio.h>
#include <ik/ik.h>

void app_main(void);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
void app_main(void);

# We apply the patch to the ik library to fix the build error.
find_package(Git REQUIRED)
execute_process(
COMMAND ${GIT_EXECUTABLE} -C ${CMAKE_CURRENT_SOURCE_DIR}/ik apply --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patch/ik_patch.patch
Copy link
Collaborator

Choose a reason for hiding this comment

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

the patch is applied to the git submodule, so it will pollute the workspace during build, right?

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