MC pos ctrl: use new uorb and params interface

[thomasgubler]

• This depends on GCC 4.7 Template Use Fix #748
• Cleanup the mc position controller to use the param and uORB interface by @jgoppert.
• No functional change intended
• This needs another round of comparing the diff by me and a live test --> not ready for merge yet but I want to make @DrTon aware of the ongoing work

[jgoppert]

This is setting the attitude subscription to update at 1 Hz. This won't work. Try setting it to 5 ms -> 200Hz.
Same thing for the other subscriptions, the rates should be corrected to match whatever was being used previously. If he wasn't explicitly setting this for the topic previously, we should choose appropriate rates.

Here are some decent guesses, if they aren't working, to get the same behaviuour as the current code you could set all of the minimum delays to 0 ms.

attitude -- 200 Hz -> 5 ms
manual -- 50 Hz -> 20 ms
control -- 50 Hz -> 20 ms
arming -- 2 Hz -> 500 ms
global_pos -- 10 Hz -> 100 ms

[LorenzMeier]

@jgoppert @thomasgubler Please also ensure that there is no aliasing - when you ask uORB to limit for 5 ms and the message arrived at 4.98 ms it will drop it, so for a 200 Hz limit and a 200 Hz signal you can end up due to jitter with 150 Hz outputs. Better to limit to 250 Hz / 4 ms if you want 200. You need to allow the system a reasonable amount of a few microseconds jitter (which is really low).

[DrTon]

I don't understand why we need topics rate limiting here? If you want to run mainloop at lower rate just use sleep and get rate that you need. Position control is maybe not so latency critical as attitude and should be ok. Another solution is to synchronize to position updates without any rate limiting for minimum latency. Don't need to limit other topics at all (attitude, manual etc.), because if synchronize to only one topic others will be polled with the same interval.

I haven't reviewed this fully, but I'm working on big upgrade of mp_pos_control now (#753) and it will have a lot of conflicts with this branch. Please, leave it as is now, I will rewrite it using blocks later myself.

[jgoppert]

Well hopefully @thomasgubler gave you a good starting point. If more people start using the C++ wrappers we can reduce a lot of boiler plate code and reduce the required programming memory quite a bit. I am not sure of the guts of the uORB topic rate limiting, but I assume if you set the arming topic update rate limit to 0 for instance then you are wasting some cycles getting new information into your sub that you don't really care about.

[DrTon]

Yes, wrappers are good, I just thought about this few days ago. I will rewrite mpc anyway. What about rate limiting: overhead from one more orb_check is not large at all, and it's better to have fresh data and waste 0.02% CPU, than have problems with topics desynchronizetion IMO.

[thomasgubler]

@jgoppert

This is setting the attitude subscription to update at 1 Hz

Sorry, I misread the argument as frequency. This is obviously wrong.
I wanted to ask you anyway about it: can we have a constructor for the Subscription class which does not set a limit?

[DrTon]

Skipping few updates will have the same effect as some polling-induced latencies, isn't it? I.e. after you skipped 10 updates, latency for 11 update will not make any sence.

For latency critical apps, of couse poll() is better, and I use it in mc_att_control to synchronize attitude controller to attitude updates.

Anyway these things can be tested and we can try both options and see if it affects prformance.

[thomasgubler]

I wanted to ask you anyway about it: can we have a constructor for the Subscription class which does not set a limit?

ah, I just saw your previous comment on this regarding usage of the 0 argument

[LorenzMeier]

@DrTon Latency and rate are not the same thing, in particular in presence of noise. If you have a short-term disturbance and zero latency, the controller will counter-act it. If you have latency, it might actually amplify it, because its compensation comes too late. If you sample slower (e.g. 100 Hz instead of 200 Hz) you might not reject the disturbance because you've missed it, but you won't amplify it.

For the case of our system, if we had in every control loop polling (which we don't have, and I know you are aware of that and already designing it right), we would easily accumulate another 20-30 ms of latency, compared to 2-5 ms latency that an asynchronous design can achieve.

I want to properly log this for over a year already - this is something we should do some time soon, to actually prove it does what we think it does 8).

[jgoppert]

@thomasgubler It would be easy to add a default value of 0 to the Subscription class, we could do something like this:

template<class T>
Subscription<T>::Subscription(
    List<SubscriptionBase *> * list,
    const struct orb_metadata *meta, unsigned interval = 0) :
}

But this would lead to more people setting the minimum delay to 0, which might not be the best.

[LorenzMeier]

@DrTon @thomasgubler Have you come to a conclusion if this is wanted or not?

[DrTon]

I'm agree with everything except MPC :) Because I have a big update for it (mpc_local_pos branch) and need to port this changes to new version. Also I want to try to improve usability of templates but first I want to complete some high priority work.