Ap_Scheduler: initialize _active_loop_rate_hz in loop() and remove check in get_loop_rate_hz()

[neo-0007]

• issue: #30234
• an extra check was made on every call of get_loop_rate_hz() for _active_loop_rate_hz == 0
• remove extra check and initialize the value of _active_loop_rate_hz in AP_Scheduler::loop()

Testing

• succesfully build for sitl
• tested on sitl

[Ryanf55]

Code changes look good. I'll rely on autotest for this.

[peterbarker]

@neo-0007 please avoid adding Merge commits to your branch. If you want to udpate it, make sure to select "Update with Rebase".

[peterbarker]

Closes https://github.com/ArduPilot/ardupilot/issues/30234

[neo-0007]

Hi @peterbarker , I have cleaned up my branch by removing the merge commits and rebased it onto the latest master as requested. I am now working on applying the suggested changes and will push them shortly!

[neo-0007]

@peterbarker I've squashed the commits into one and pushed the updated version with the requested changes. Let me know if anything else is needed.

[tpwrules]

#31138 should fix the need for the extra set, but there is some more cleanup to do. Are you interested in further work on this?

[neo-0007]

#31138 should fix the need for the extra set, but there is some more cleanup to do. Are you interested in further work on this?

Sure! I’ll start with #31138. Could you outline the cleanup tasks you have in mind? If you can suggest the next set of changes, I’ll pick them up and start working on them.

[tpwrules]

If you rebase this PR on top of master now that PR will be included.

The changes in my mind:

• The variables are already set up in https://github.com/ArduPilot/ardupilot/blob/1e8180d2d257983c94fd5f2d68dad465821a02d5/libraries/AP_Scheduler/AP_Scheduler.cpp#L116 so we don't need to set them up in loop()
• The comment there should be clarified to say that these are the only place those variables are initialized
• The related inits in the header in get_loop_period_us() and get_loop_period_s() can also be dropped
• This could probably be dropped https://github.com/ArduPilot/ardupilot/blob/1e8180d2d257983c94fd5f2d68dad465821a02d5/ArduPlane/Plane.cpp#L386-L391

Thanks for your effort and understanding!

[neo-0007]

I simplified the getters and made them const

    // get the active main loop rate
    uint16_t get_loop_rate_hz(void) const {
        return _active_loop_rate_hz;
    }
    // get the time-allowed-per-loop in microseconds
    uint32_t get_loop_period_us() const {
        return _loop_period_us;
    }
    // get the time-allowed-per-loop in seconds
    float get_loop_period_s() const {
        return _loop_period_s;
    }

This could probably be dropped

 // this ensures G_Dt is correct, catching startup issues with constructors 
 // calling the scheduler methods 
 if (!is_equal(1.0f/scheduler.get_loop_rate_hz(), scheduler.get_loop_period_s()) || 
     !is_equal(G_Dt, scheduler.get_loop_period_s())) { 
     INTERNAL_ERROR(AP_InternalError::error_t::flow_of_control); 
 }

Should i simplify this check or remove it completly ?

[tpwrules]

I simplified the getters and made them const

Looks great!

 // this ensures G_Dt is correct, catching startup issues with constructors 
 // calling the scheduler methods 
 if (!is_equal(1.0f/scheduler.get_loop_rate_hz(), scheduler.get_loop_period_s()) || 
     !is_equal(G_Dt, scheduler.get_loop_period_s())) { 
     INTERNAL_ERROR(AP_InternalError::error_t::flow_of_control); 
 }

Should i simplify this check or remove it completly ?

Remove it completely.

[neo-0007]

@tpwrules I implemented the suggested changes and also included some changes previously mentioned by @peterbarker.

I’ve opened PR #31165 — I’d appreciate a review and any further suggestions. Happy to make adjustments as needed. Thanks!

[tpwrules]

You shol