search

nobleo / path_tracking_pid

Path Tracking PID offers a tuneable PID control loop, decouling steering and forward velocity
Apache License 2.0
129 stars 38 forks source link

Preliminary boost units implementation. #72

Closed lewie-donckers closed 2 years ago

lewie-donckers commented 2 years ago

Preliminary boost units implementation. No proper review required yet. This is just to show what it could look like.

It contains the same changes to the interfaces and calculations as #70 but this time using the Boost units library.

This is currently quite verbose but by adding a few typedefs we could reduce that.

lewie-donckers commented 2 years ago

I noticed that this does not build on melodic. That compiler does not support the Boost units as constexpr values it seems.

No doubt, we could work around this, if required.

Timple commented 2 years ago

Lets not spend any effort on melodic until we decide to do so.

rokusottervanger commented 2 years ago

Lets not spend any effort on melodic until we decide to do so.

We currently still have a customer using this controller under melodic. We're trying our best to get to move away from the system they're using that limits them from upgrading to noetic, but currently we still need to support this.

That's not to say that we need to fix this build right away, of course. Only if we decide to apply Boost units. I assume that's what you meant, @Timple ?

Timple commented 2 years ago

Yup, if we decide to use boost units, we should make the tradeoff between the two efforts: Upgrading all melodic robots vs supporting melodic.

Although the latter will probably 'win'.

lewie-donckers commented 2 years ago

I found the reason the build failed on Melodic. The standard Docker container for Melodic uses Boost 1.65.1. (It is based on Ubuntu 18.04 I believe.)

The Boost Units library only got constexpr support in Boost 1.67 (somewhere in 2018). So not using constexpr but const for unit based constants is a workaround for the issue.

Timple commented 2 years ago

Well that was an easy fix. const it is then!

Personally I like the boost implementation better than a self-written one. Less prone for errors in the implementation, more chance of re-use in other packages.

So as far as I'm concerned we finish this implementation in favor of #70

lewie-donckers commented 2 years ago

I will close both PRs but keep the branches around for a bit longer.