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

Update sdk bindings #59

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update sdk bindings #59

wants to merge 1 commit into from

Conversation

VHolonec
Copy link
Contributor

This PR is made for tof sdk from dev-5.1.0 branch.

@VHolonec VHolonec requested a review from rbudai98 April 30, 2024 07:27
@@ -53,7 +53,7 @@ std::string * parseArgs(int argc, char ** argv);
std::shared_ptr<aditof::Camera> initCamera(std::string * arguments);
void startCamera(const std::shared_ptr<aditof::Camera> & camera);
void stopCamera(const std::shared_ptr<aditof::Camera> & camera);
void setFrameType(const std::shared_ptr<aditof::Camera> & camera, const std::string & type);
void setCameraMode(const std::shared_ptr<aditof::Camera> & camera, const std::string & type);
Copy link
Contributor

Choose a reason for hiding this comment

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

type->mode
Also, latest dev-5.1.0 uses int type for mode

return;
} else {
LOG(INFO) << "Frame type set: " << type;
LOG(INFO) << "Frame mode set: " << mode;
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be "Camera mode" not "Frame mode"

@VHolonec VHolonec force-pushed the tofcmos-703-update-bindings branch 2 times, most recently from c278a9c to bd5cb7d Compare April 30, 2024 11:46
rbudai98
rbudai98 previously approved these changes May 7, 2024
void getAvailableFrameTypes(
const std::shared_ptr<aditof::Camera> & camera, std::vector<std::string> & availableFrameTypes)
void getAvailableModes(
const std::shared_ptr<aditof::Camera> & camera, std::vector<uint8_t> & availableModes)
{
//get available frae types of camera
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment to mode

@@ -246,17 +246,17 @@ int main(int argc, char * argv[])
}

// getting available frame types, backward compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

similarly here

@rbudai98 rbudai98 dismissed their stale review May 7, 2024 06:43

Even though build works, the node does not start

@rbudai98
Copy link
Contributor

rbudai98 commented May 7, 2024

2 more things need checking:

  • main README needs update with the modes
  • with tof:dev-5.1.0 branch the node still stopps at segmentation fault

@VHolonec VHolonec force-pushed the tofcmos-703-update-bindings branch from bd5cb7d to c68ebcb Compare May 7, 2024 07:32
@SeptimiuVana
Copy link

Remove old config files from tof_config since they are no longer used.
https://github.com/analogdevicesinc/tof-ros2/tree/tofcmos-703-update-bindings/tof_config

@@ -53,13 +53,13 @@ std::string * parseArgs(int argc, char ** argv);
std::shared_ptr<aditof::Camera> initCamera(std::string * arguments);
void startCamera(const std::shared_ptr<aditof::Camera> & camera);
void stopCamera(const std::shared_ptr<aditof::Camera> & camera);
void setFrameType(const std::shared_ptr<aditof::Camera> & camera, const std::string & type);
void setCameraMode(const std::shared_ptr<aditof::Camera> & camera, const uint8_t & mode);

Choose a reason for hiding this comment

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

Do we need both setMode and setCameraMode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed setMode because is the same as setCameraMode

Choose a reason for hiding this comment

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

Do we use this image somewhere?

@VHolonec VHolonec force-pushed the tofcmos-703-update-bindings branch 2 times, most recently from de78658 to c61dc21 Compare May 7, 2024 11:04
@@ -1,4 +1,4 @@
{
"depthIni": "./config/RawToDepthAdsd3030_sr-native.ini;./config/RawToDepthAdsd3030_lr-native.ini;./config/RawToDepthAdsd3030_sr-qnative.ini;./config/RawToDepthAdsd3030_lr-qnative.ini;./config/RawToDepthAdsd3030_sr-mixed.ini;./config/RawToDepthAdsd3030_lr-mixed.ini;./config/RawToDepthAdsd_pcm-native.ini",
"depthIni": "./config/RawToDepthAdsd3030_0.ini;./config/RawToDepthAdsd3030_1;./config/RawToDepthAdsd3030_2.ini;./config/RawToDepthAdsd3030_3.ini;./config/RawToDepthAdsd3030_6.ini;./config/RawToDepthAdsd3030_5.ini;./config/RawToDepthAdsd_4.ini",

Choose a reason for hiding this comment

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

/config/RawToDepthAdsd3030_1 is missing .ini extension.

@VHolonec VHolonec force-pushed the tofcmos-703-update-bindings branch from c61dc21 to 06abc94 Compare May 8, 2024 06:52
@VHolonec
Copy link
Contributor Author

VHolonec commented May 8, 2024

Do not merge this PR until branch dev-5.1.0 from tof is merged into main.

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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