Skip to content
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

Expose PID coefficients and values over dbus #3

Open
pstrinkle opened this issue Aug 30, 2018 · 3 comments
Open

Expose PID coefficients and values over dbus #3

pstrinkle opened this issue Aug 30, 2018 · 3 comments
Assignees

Comments

@pstrinkle
Copy link
Member

Currently we expose a zone's state, whether it's in failsafe or manual
mode. One can read or write this information over IPMI via OEM
commands. With the PID objects, it may also be helpful to read the
PID values over IPMI periodically (or later read/write them).

A zone has PIDs, so we can expose an interface on each PID that
exports the values onto dbus. Obviously, values that are updated
regularly should only be updated in dbus if someone tries to read them
-- that's what I'm thinking there. Keep it more of a report on read
interface instead of one that broadcasts for others.

I'm going to start hacking out an initial design over the next few
days and update this email thread.

Let me know if anyone is interested in this information or has already
worked it out.

objectPath = "/xyz/openbmc_project/settings/fanctrl/zone{zoneid}";
busName = "xyz.openbmc_project.State.FanCtrl";
intf = "xyz.openbmc_project.Control.Mode";

That has that interface.

We may want to have other interfaces, and objects on the same bus.
One for each PID.
Each zone has its own PIDs.. PIDs have their own IDs.

typedef struct {
bool initialized; // has pid been initialized

float       ts;                 // sample time in seconds
float       integral;           // intergal of error
float       last_output;        // value of last output

float       p_c;                // coeff for P
float       i_c;                // coeff for I
float       ff_off;             // offset coeff for feed-forward term
float       ff_gain;            // gain for feed-forward term

limits_t    i_lim;              // clamp of integral
limits_t    out_lim;            // clamp of output
float       slew_neg;
float       slew_pos;

} pid_info_t;

ec::pid_info_t _pid_info;
float _setpoint;
std::string _id;

@pstrinkle pstrinkle self-assigned this Aug 30, 2018
@feistjj
Copy link
Member

feistjj commented Aug 30, 2018

image

Just as an FYI the entity-manager configuration shows most of this information on d-bus, although I know it's not exactly what you're going for here.

@pstrinkle
Copy link
Member Author

Yeah, that's how y'all are configuring it over dbus. As you said, what I'm describing is a bit different -- the current values etc, exposed over an interface.

@Krellan
Copy link
Member

Krellan commented May 16, 2023

We expose the PID coefficients now, indirectly, as the same Inventory objects from entity-manager which are already used as input for phosphor-pid-control configuration.

This does nothing to help expose PID coefficients for the old hardcoded config.json code path, though.

Would it also be beneficial to expose the PID coefficients in an additional place, read-only, showing what the phosphor-pid-control program currently believes them to be? This would help diagnose bugs in which the configuration gets out of sync, such as when they are changed in entity-manager but phosphor-pid-control fails to pick up these changes for any reason.

Also, entity-manager is only for the configuration available at startup time. The additional information available at runtime, such as the computed input/output/setpoint of each PID loop, are not exposed yet. These would be very useful to expose from within phosphor-pid-control, for debugging and thermal tuning purposes. This is further discussed here: #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants