diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 816b76cb83f..c07b51d35f4 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -252,7 +252,15 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_bdp_estimator_init(&t->bdp_estimator); t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); t->last_pid_update = t->last_bdp_ping_finished; - grpc_pid_controller_init(&t->pid_controller, log2(DEFAULT_WINDOW), 4, 4, 0); + grpc_pid_controller_init( + &t->pid_controller, + (grpc_pid_controller_args){.gain_p = 4, + .gain_i = 8, + .gain_d = 0, + .initial_control_value = log2(DEFAULT_WINDOW), + .min_control_value = -1, + .max_control_value = 22, + .integral_range = 10}); grpc_chttp2_goaway_parser_init(&t->goaway_parser); grpc_chttp2_hpack_parser_init(&t->hpack_parser); diff --git a/src/core/lib/transport/pid_controller.c b/src/core/lib/transport/pid_controller.c index ba568745039..3a4845d7ef8 100644 --- a/src/core/lib/transport/pid_controller.c +++ b/src/core/lib/transport/pid_controller.c @@ -32,14 +32,12 @@ */ #include "src/core/lib/transport/pid_controller.h" +#include void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double initial_control_value, double gain_p, - double gain_i, double gain_d) { - pid_controller->last_control_value = initial_control_value; - pid_controller->gain_p = gain_p; - pid_controller->gain_i = gain_i; - pid_controller->gain_d = gain_d; + grpc_pid_controller_args args) { + pid_controller->args = args; + pid_controller->last_control_value = args.initial_control_value; grpc_pid_controller_reset(pid_controller); } @@ -53,13 +51,20 @@ double grpc_pid_controller_update(grpc_pid_controller *pid_controller, /* integrate error using the trapezoid rule */ pid_controller->error_integral += dt * (pid_controller->last_error + error) * 0.5; + pid_controller->error_integral = GPR_CLAMP( + pid_controller->error_integral, -pid_controller->args.integral_range, + pid_controller->args.integral_range); double diff_error = (error - pid_controller->last_error) / dt; /* calculate derivative of control value vs time */ - double dc_dt = pid_controller->gain_p * error + - pid_controller->gain_i * pid_controller->error_integral + - pid_controller->gain_d * diff_error; + double dc_dt = pid_controller->args.gain_p * error + + pid_controller->args.gain_i * pid_controller->error_integral + + pid_controller->args.gain_d * diff_error; + /* and perform trapezoidal integration */ double new_control_value = pid_controller->last_control_value + dt * (pid_controller->last_dc_dt + dc_dt) * 0.5; + new_control_value = + GPR_CLAMP(new_control_value, pid_controller->args.min_control_value, + pid_controller->args.max_control_value); pid_controller->last_error = error; pid_controller->last_dc_dt = dc_dt; pid_controller->last_control_value = new_control_value; diff --git a/src/core/lib/transport/pid_controller.h b/src/core/lib/transport/pid_controller.h index dd2b1200525..1bf2fbc5640 100644 --- a/src/core/lib/transport/pid_controller.h +++ b/src/core/lib/transport/pid_controller.h @@ -45,16 +45,23 @@ typedef struct { double gain_p; double gain_i; double gain_d; + double initial_control_value; + double min_control_value; + double max_control_value; + double integral_range; +} grpc_pid_controller_args; + +typedef struct { double last_error; double error_integral; double last_control_value; double last_dc_dt; + grpc_pid_controller_args args; } grpc_pid_controller; /** Initialize the controller */ void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double initial_control_value, double gain_p, - double gain_i, double gain_d); + grpc_pid_controller_args args); /** Reset the controller: useful when things have changed significantly */ void grpc_pid_controller_reset(grpc_pid_controller *pid_controller); diff --git a/test/core/transport/pid_controller_test.c b/test/core/transport/pid_controller_test.c index 3935a25322b..af53d5b8cb0 100644 --- a/test/core/transport/pid_controller_test.c +++ b/test/core/transport/pid_controller_test.c @@ -33,6 +33,7 @@ #include "src/core/lib/transport/pid_controller.h" +#include #include #include @@ -45,7 +46,14 @@ static void test_noop(void) { gpr_log(GPR_INFO, "test_noop"); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, 0, 1, 1, 1); + grpc_pid_controller_init( + &pid, (grpc_pid_controller_args){.gain_p = 1, + .gain_i = 1, + .gain_d = 1, + .initial_control_value = 1, + .min_control_value = DBL_MIN, + .max_control_value = DBL_MAX, + .integral_range = DBL_MAX}); } static void test_simple_convergence(double gain_p, double gain_i, double gain_d, @@ -55,7 +63,14 @@ static void test_simple_convergence(double gain_p, double gain_i, double gain_d, "start=%lf", gain_p, gain_i, gain_d, dt, set_point, start); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, start, 0.2, 0.1, 0.1); + grpc_pid_controller_init( + &pid, (grpc_pid_controller_args){.gain_p = gain_p, + .gain_i = gain_i, + .gain_d = gain_d, + .initial_control_value = start, + .min_control_value = DBL_MIN, + .max_control_value = DBL_MAX, + .integral_range = DBL_MAX}); for (int i = 0; i < 1000; i++) { grpc_pid_controller_update(&pid, set_point - grpc_pid_controller_last(&pid),