move MC gyro subscription + corrections to BlockGyroCorrected #9248
Conversation
|
Later we can cleanup the structure so that only the rate controllers can run faster than attitude. @priseborough FYI |
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
2 times, most recently
from
bb16462 to
29fb788
Compare
|
Great stuff! <3 |
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
2 times, most recently
from
7d326ef to
8a9136e
Compare
|
@mhkabir @RomanBapst this should be good to go after a quick MC and FW (or tailsitter) verification. |
dagar
changed the title
|
|
||
| private: | ||
|
|
||
| control::BlockParamFloat _sens_board_rot; |
There was a problem hiding this comment.
Can you please use the new param class?
There was a problem hiding this comment.
Any issues with killing off BlockParam entirely? I was trying to keep controllib consistent.
There was a problem hiding this comment.
That was my next step, yes. And use dependency injection for the controllib classes that require params.
Since it's always the same set here, you can directly use the board rotation params.
| _board_rotation = offset * get_rot_matrix((enum Rotation)_sens_board_rot.get()); | ||
| } | ||
|
|
||
| void update() { updateParams(); } |
|
|
||
| static constexpr int MAX_GYRO_COUNT = 3; | ||
|
|
||
| class __EXPORT BlockGyroCorrected : public SuperBlock |
| int _sensor_bias_sub{-1}; /**< sensor in-run bias correction subscription */ | ||
|
|
||
|
|
||
| void update_bias(); |
There was a problem hiding this comment.
can you move the methods above the private attributes?
There was a problem hiding this comment.
Yes, but if you have these kinds of preferences can we either put some rough style notes together or reference something that's close enough?
There was a problem hiding this comment.
It's according to the google style: https://google.github.io/styleguide/cppguide.html#Declaration_Order. And it's also how most projects I came across do it.
| const matrix::Vector3f &get() { return _rates; } | ||
| const uint64_t ×tamp() { return _timestamp; } | ||
|
|
||
| bool poll(int timeout = 100); |
There was a problem hiding this comment.
missing documentation:
- unit of timestamp
- it's not obvious that this will call
update()
There was a problem hiding this comment.
I was thinking of changing it update blocking or something rather than overloading something called poll.
There was a problem hiding this comment.
That's fine with me. It's mostly about it being easy to understand & use
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
3 times, most recently
from
c2988d2 to
c771ac1
Compare
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
from
c771ac1 to
e1ac387
Compare
|
This is a case where parameter scope gets a little weird. The sensors module still "owns" the board rotation, but it's only actually used in controllib. |
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
from
e1ac387 to
b376061
Compare
dagar
force-pushed
the
pr-sensor_gyro_controllib
branch
from
b376061 to
945f8e4
Compare
|
I have a different idea for handling this that I'll explore once #8814 is in place. |
Controllib seemed like an okay place to put this, but I'm certainly open to suggestions.