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

Add time parameterization interface and optional implementation builds #254

Closed
marip8 opened this issue Oct 10, 2022 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@marip8
Copy link
Contributor

marip8 commented Oct 10, 2022

It seems like all 3 existing implementations of time parameterization are intended to perform the same function. We should create an abstract interface for time parameterization and add the ability to optionally disable builds of implementations that need additional 3rd party dependencies (e.g., Ruckig)

@Levi-Armstrong
Copy link
Contributor

I agree. We can still make them optional to build now if that is something needed now.

@Levi-Armstrong Levi-Armstrong added the enhancement New feature or request label Feb 28, 2023
@marip8
Copy link
Contributor Author

marip8 commented Nov 6, 2023

Optional builds added in #373

@Levi-Armstrong
Copy link
Contributor

@marip8 Are you working on the interface?

@marip8
Copy link
Contributor Author

marip8 commented Nov 7, 2023

Currently each algorithm has a slightly different signature for what it can/does support:

Shared

  • max_velocity_scaling_factor (single value)
  • acceleration_scaling_factor (single_value)
  • max_velocity (single value and per joint)
  • max_acceleration (single value and per joint)

Different

  • max_velocity_scaling_factor (per waypoint - ISP, Ruckig)
  • max_acceleration_scaling_factor (per waypoint - ISP, Ruckig)
  • max_jerk (single value and per joint - Ruckig)
  • max_jerk_scaling_factor (single value, per waypoint - Ruckig)

Does the per-waypoint max velocity/acceleration scaling factor actually function correctly? My experience was that it doesn't for ISP. If that is accurate, maybe we just drop that interface.

As far as jerk scaling, I think we would need to add it to the interface and just ignore the values for ISP and TOTG

@Levi-Armstrong
Copy link
Contributor

Can we make the interface simpler so these are not part of the interface but just part of the yaml config/profiles?

@marip8
Copy link
Contributor Author

marip8 commented Nov 7, 2023

Can we make the interface simpler so these are not part of the interface but just part of the yaml config/profiles?

Depends. We could do this suggestion by just moving the algorithms into their respective tasks in task composer (like in SNP with the constant TCP speed time paramterization classes). I guess the question is whether or not there is value in having a stand-alone time parameterization class that exists outside of task composer. If there is value there, then it would make sense to create a common interface and force all implementations to provide that interface.

I personally only have only used the time parameterization algorithms as a part of a task pipeline, so I might lean towards dissolving tesseract_time_parameterization into tesseract_task_composer and creating libraries for the algorithms themselves in case anyone wants to use want for a special use-case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants
@Levi-Armstrong @marip8 and others