-
Notifications
You must be signed in to change notification settings - Fork 40
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
Comments
I agree. We can still make them optional to build now if that is something needed now. |
Optional builds added in #373 |
@marip8 Are you working on the interface? |
Currently each algorithm has a slightly different signature for what it can/does support: Shared
Different
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 |
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 |
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)
The text was updated successfully, but these errors were encountered: