-
Notifications
You must be signed in to change notification settings - Fork 17.2k
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
Start moving bulk amounts of init_ardupilot to base class #13299
Start moving bulk amounts of init_ardupilot to base class #13299
Conversation
@peterbarker I had to add some more generic vehicle stuff for my FFT PR: |
On Thu, 16 Jan 2020, Andy Piper wrote:
@peterbarker I had to add some more generic vehicle stuff for my FFT PR:
84a6f26
48e551d
7c878f1
Feel free to cherry-pick if it would help
That seems to be an alternate way of getting things hooked into the parent
class object. We could add an arbitrary number of such hooks.
I *think* the equivalent set of patches after my PR would be adding a
three-line entry at the bottom of setup() after init_ardupilot is called -
if I'm thinking right.
|
On Thu, 16 Jan 2020, Andy Piper wrote:
@peterbarker I had to add some more generic vehicle stuff for my FFT PR:
84a6f26
48e551d
7c878f1
Feel free to cherry-pick if it would help
I think mine is pretty much an alternate way of achieving the same result.
I *think* the equivalent patch on top of my PR would be to add the
three-line hook into setup() below init_ardupilot().
The existing init_vehicle hook should also be collapsed back into the
moved-up setup() call on top of this PR - at the moment, being hooked from
boardconfig, it's being run at random times in the sequence; in Sub it is
(erroneously IMO) run really quite early, in other vehicles quite a bit
later. Didn't feel the need to complicate this PR with that...
|
@peterbarker the FFT stuff has to initialize late as it needs the IMUs to be initialized and started. I guess if the IMU initialization moves here then that would help as I could insert the FFT stuff after. The boardconfig init is really early, its going to cause problems for a while I think |
@andyp1per https://github.com/ArduPilot/ardupilot/pull/13299/files#diff-021e65608195c65fc50f136f63dad9cdR96 <- that's really, really late :-) I honestly don't know if we have to run the scheduler initialisation as late as we do. My guess is, "no" - but I put it there to preserve behaviour. IOW, we could even move stuff to after the scheduler init if you want it later still :-) |
@peterbarker having the scheduler start late is a good thing I think. I've had to write too much defensive code for coping with the scheduler started but my init() method not being called! |
On Thu, 16 Jan 2020, Andy Piper wrote:
@peterbarker having the scheduler start late is a good thing I think. I've had to write too much defensive code for coping with the scheduler started but
my init() method not being called!
This is actually scheduler initialisation, rather than actually running
the scheduler. So kinda-sorta "started"? :-)
|
de33660
to
65751da
Compare
what does this do to firmware size for Pixhawk1-1M ? |
In general I think this is a great thing to do. |
65751da
to
0d86680
Compare
We don't need the flexibility to reset this, it's a waste of bytes and something that could go wrong. AP_Periph led the way with using pragmas here.
This makes it the same as Rover and Copter.
~70 bytes larger |
0d86680
to
e6fb070
Compare
Parameters, mission and fence are preserved across the move from master to this branch. This validates the StorageManager changes. |
I flew this on my 250. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The change on copter parameters table where already well tested on #10391
Close #10391 |
A rather large amount of code can be moved from the
init_ardupilot
into the base-classsetup
library.I've restricted the movement in this PR to just a few functions as they're relatively straight-forward.
Obtaining our list of scheduled tasks might be improved in a future PR; perhaps something along the same lines as how we do the logging structures might work.