Fix regex in findSensors

swampd crashs while reading configuration. Root caused that it is
introduced by incorrect finding sensors. For an example, when
it searches 'Fan_1' for inputs, it finds thes

Fix regex in findSensors

swampd crashs while reading configuration. Root caused that it is
introduced by incorrect finding sensors. For an example, when
it searches 'Fan_1' for inputs, it finds these two sensors:

Fan_1
Pwm_PSU1_Fan_1

where 'Pwm_PSU1_Fan_1' is an unexpected search result, so it causes crash
after printing out this message:

"sensor at dbus path [/xyz/openbmc_project/control/fanpwm/Pwm_PSU1_Fan_1]
has an interface [xyz.openbmc_project.Control.FanPwm] that does not match
the expected interface of xyz.openbmc_project.Sensor.Value"

To fix this issue, this commit modifies regex string in findSensors
function.
Unit test is updated to reflect the new behavior.

Tested:
The crash was not observed.
Updated unit test passed.

Signed-off-by: Zhikui Ren <zhikui.ren@intel.com>
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Change-Id: I4e45fac3a3242a6f3655d6873fd63ef22fd0c4dd

show more ...

Bug fix: pidControlLoop may loop infinitely.

boost::basic_waitable_timer::cancel is a non-blocking function and only
sends cancellation to the pending operations. For other operations:
```
If the ti

Bug fix: pidControlLoop may loop infinitely.

boost::basic_waitable_timer::cancel is a non-blocking function and only
sends cancellation to the pending operations. For other operations:
```
If the timer has already expired when cancel() is called, then the
handlers for asynchronous wait operations will:
* have already been invoked; or
* have been queued for invocation in the near future.
These handlers can no longer be cancelled, and therefore are passed an
error code that indicates the successful completion of the wait
operation.
```

In our case, if pidControlLoop() has been invoked or in the invoke
queue while the timer cancellation, it will ignore the cancal ec and
infinitely call the pidControlLoop() chain.

Thus an extra cancel variable is introduced to break the chain.

Signed-off-by: Hao Jiang <jianghao@google.com>
Change-Id: Ie4e53454ee326bdf612abb511990610a6b528300

show more ...

Expose exception of restartControlLoops().

Expose the failure of restartControlLoops() for the last loop. The
orignal rethrowing an exception will lose the call stack infomation.
It leads to extreme

Expose exception of restartControlLoops().

Expose the failure of restartControlLoops() for the last loop. The
orignal rethrowing an exception will lose the call stack infomation.
It leads to extreme difficulties when debugging.

Comparing to embedding the traceback infomation into the new (nested)
exception, I would rather expose the exception directly to external
which preserves both call stack and varibale information.

Signed-off-by: Hao Jiang <jianghao@google.com>
Change-Id: I1535fc5768ea58e5af695948eec65fdfa84e8ff9

show more ...

Retry pidControlLoop in individual context handler

io.context is non-preemptive on single thread, which means if all
restartControlLoops are scheduled within one handler, the resources
which are hel

Retry pidControlLoop in individual context handler

io.context is non-preemptive on single thread, which means if all
restartControlLoops are scheduled within one handler, the resources
which are held by pidControlLoop will never be released even given that
their contexts have been cancelled, since their contexts cannot preempt
the context of restartControlLoops.

We need to schedule restartControlLoops in individual context handlers.
Thus when cancellation signal is broadcasted, context runtime will be
yield from restartControlLoops to pidControlLoop to release resources.

Tested:
With multiple reboot cycle (2 machines with 10 reboot cycle with
interval of 10 min), swampd is stable and functional.

Change-Id: I0c750dcfe73016b71c6cfd693c4f8ed99b12e5d0
Signed-off-by: Hao Jiang <jianghao@google.com>

show more ...

Modify function signature for unit test.

sdbusplus changed the function signature of
sd_bus_emit_properties_changed_strv. Modify unit test according.

Signed-off-by: Hao Jiang <jianghao@google.com>

Modify function signature for unit test.

sdbusplus changed the function signature of
sd_bus_emit_properties_changed_strv. Modify unit test according.

Signed-off-by: Hao Jiang <jianghao@google.com>
Change-Id: I372543425db822ad4c8c48b5f93a72f291d69d11

show more ...

MAINTAINERS: Venture: M->R

Due to lack of dedicated time, demoting myself from maintainer to
interested party.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Ib5d19c1149422b2f19bb33

MAINTAINERS: Venture: M->R

Due to lack of dedicated time, demoting myself from maintainer to
interested party.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Ib5d19c1149422b2f19bb33cde009a63b584d427c

show more ...

Add Ed as a maintainer

Patrick reached out to me with a desire to get more depth in
maintainership on phosphor-pid-control, so here I am. I have very few
patchsets against PPC, but I maintain a lot

Add Ed as a maintainer

Patrick reached out to me with a desire to get more depth in
maintainership on phosphor-pid-control, so here I am. I have very few
patchsets against PPC, but I maintain a lot of components around it, so
I'm happy to help, and I understand the form and function pretty well.

I really would've preferred pulling a sword from a stone, or been given a
2 factor encryption key by the lady of the lake, but as monty python
says "strange women lying in ponds distributing swords is no basis for a
system of government. Supreme executive power derives from a mandate
from the masses, not from some farcical aquatic ceremony."

As such, I submit this to the masses for approval.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iaac047efd504dc100bcf9d2eec0dd7c77e3f1674

show more ...

FanController/ThermalController: Clean up PID input math

Adding checking against floating-point oddities,
such as NAN, +INF, or -INF, appearing as input,
which would mess up the PID loop math,
causi

FanController/ThermalController: Clean up PID input math

Adding checking against floating-point oddities,
such as NAN, +INF, or -INF, appearing as input,
which would mess up the PID loop math,
causing corrupt output.

If a fan or thermal input value is NAN, +INF, or -INF,
that value will be omitted from contributing to PID loop input.
If all values were omitted for that PID loop,
existing code already hardcodes a value of 0 for fan,
and I added similar code to also hardcode 0 for thermal.

It makes sense to use a placeholder value of 0 degrees of margin,
because this will make the fans spin fast,
if for some reason the zone is not already in failsafe mode by now.
Note that negative values are not allowed for fan,
but they are allowed for thermal.

Tested: Works for me, and PID loops no longer output garbage
when debugging a sensor malfunction while in tuning mode,
making tuning mode much more usable. If not in tuning mode,
the normal failsafe feature would have kicked in,
masking this garbage from appearing in the output anyway.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I7aee812ebaeff209f84cef0db28973696f782ef9

show more ...

Remove James from MAINTAINERS

bye!

Change-Id: Ia6909f6b542baba3849670862bd2172faba3d44f
Signed-off-by: James Feist <james.feist@linux.intel.com>

drop struct keyword for non-packed objects

As a style decision, struct is often used with packed structures to
indicate they are used like C-structs. Cleanup this codebase to not use
the extra stru

drop struct keyword for non-packed objects

As a style decision, struct is often used with packed structures to
indicate they are used like C-structs. Cleanup this codebase to not use
the extra struct keyword throughout.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I2f83bb1989e4d1f2f843ba3e45fb82e04f0fa61c

show more ...

dbus: parameterize dbusconfiguration init

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I9cb6c0a19d4625d79da8613bd6eea13d6cc04220

dbus: move debugPrint to util

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Idbea54f13527a4ca3108cc4dccbee07b7b3a7d34

dbus: move zone indexing to util

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Ia6560a0e0f976dcf9f05004de68f74f5e313950b

dbus: Move findSensors to dbus util

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I2ba4d3c430200eb58cb4364dcce694a080531846

dbus: add unit-tests for dbusutil: getSensorPath

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Iabb2df7a725da1b70cb6a907d19c451e7bba609d

main: Allowing logging and tuning to also be enabled by files

The logging and tuning flags, which come from the command line,
are now also able to be enabled by the presence of certain files.

This

main: Allowing logging and tuning to also be enabled by files

The logging and tuning flags, which come from the command line,
are now also able to be enabled by the presence of certain files.

This allows the logging and tuning features to be enabled,
at runtime, without having to change the command line.

The content of /etc/thermal.d/tuning does not matter,
only the existence of this file is checked for.
The content of /etc/thermal.d/logging can optionally
be a directory path, which will be used if it exists,
otherwise use the systemwide default temporary directory.

This makes it easy for people doing thermal calibration
to enable these options as they need, without having to
destabilize the system by hand-editing the systemd configuration,
which is non-trivial to change at runtime, due to systemd caching.

The directory choice of /etc/thermal.d has precedence,
as it was already hardcoded in the setpoint file during tuning.

Tested: Tuning mode was enabled and logging appeared in /tmp
mkdir -p /etc/thermal.d
touch /etc/thermal.d/logging
touch /etc/thermal.d/tuning
killall swampd

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: Ie74673664de806cbb8a0c70c61310e646ec38014

show more ...

DbusPidZone: Allow per-zone setpoint files during tuning

In tuning mode, in addition to the overall setpoint file,
also check for the existence of per-zone setpoint files.

If they exist, and contai

DbusPidZone: Allow per-zone setpoint files during tuning

In tuning mode, in addition to the overall setpoint file,
also check for the existence of per-zone setpoint files.

If they exist, and contain a parseable RPM number,
they will be used. If both per-zone and overall files exist,
the per-zone will override the overall, appropriately for each zone.

Better error checking has been added,
detecting common user mistake of using PWM instead of RPM,
throttling error messages to avoid repetitive output.

Tested: It worked, here's an example...
echo 5000 > /etc/thermal.d/setpoint.zone0
echo 7000 > /etc/thermal.d/setpoint
echo 9000 > /etc/thermal.d/setpoint.zone2
Zone 0 will try for 5000 RPM, Zone 2 will try for 9000 RPM,
and Zone 1 (and all other zones) will try for 7000 RPM.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: Ic78d0fd2a0c32308356a0e2c7b03453416467c5b

show more ...

dbusconfiguration: Accept ZoneIndex parameter for zoneID

When using configuration from D-Bus entity-manager,
zone ID numbers were assigned in a "randomly" haphazard way,
making it difficult to see w

dbusconfiguration: Accept ZoneIndex parameter for zoneID

When using configuration from D-Bus entity-manager,
zone ID numbers were assigned in a "randomly" haphazard way,
making it difficult to see what zone was being talked about,
during logging messages and error messages and so on.

Accept a new JSON parameter "ZoneIndex",
in the "Pid.Zone" configuration stanza (where MinThermalOutput is),
and use it appropriately as the key of the std::map variables,
when building out the data structures from the JSON.

This gives feature parity with the old config.json configuration,
which already featured the parameter "id" which accomplished this.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I8a506ff877d150ae1da5c16364a72551a2e80319

show more ...

dbus: move types to header for use in tests

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Iff106e17fa020d7deff29b78af8a5337324317d8

dbuspassive: Input processing cleanup and allow "margin"

The sensor input processing steps have been cleaned up,
consolidating code duplicated in two places into one.
The new "updateValue" function

dbuspassive: Input processing cleanup and allow "margin"

The sensor input processing steps have been cleaned up,
consolidating code duplicated in two places into one.
The new "updateValue" function does the processing,
then calls "setValue" to set it, if it was acceptable.

Initialization of sensor value now uses the same processing
path as regular sensor updates, solving a strange discontinuity
seen at initialization, where the values had wrong scaling,
until the first asynchronous D-Bus sensor update was received.

Also cleaned up handling of floating-point NAN received,
which has same meaning as old -1 sentinel value,
as used back when the D-Bus schema was integer millidegrees.

Sensors of type "margin" are now accepted,
as they are now processed correctly.

Adding new member "_typeMargin" flag to remember margin type,
avoiding repeated string computation.
Adding new member "_badReading" flag to track the case in which
sensor reading is working but gave us bad reading, such as NAN,
as this situation differs from already-existing "_failed" flag.
Adding new member "_marginHot" flag to request failsafe mode,
even though sensor is functional and reading is good,
because if we have ran out of margin, PID is not doing the job,
and thus we need failsafe mode, to make sure fans spin fast.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: Ia6e2f58967f58ae2999489e3e9342388f7b5fb1a

show more ...

dbuswrite: Add another write() form to WriteInterface

Adding another form of the write() call,
which takes 3 arguments instead of 1,
providing an alternate interface for caller to use.

A compatibil

dbuswrite: Add another write() form to WriteInterface

Adding another form of the write() call,
which takes 3 arguments instead of 1,
providing an alternate interface for caller to use.

A compatibility function is provided,
which simply calls write(), so no API breakage.

The additional arguments to write() allow caller
to force the write to happen even if old value same,
and/or to learn the actual raw value written.

Adding boilerplate to Sensor and PluggableSensor,
to also pass the 3-argument write() through.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: Iaf35f674ef9ce38b56017f6cb0f821df6c789649

show more ...

Add Josh Lehan to MAINTAINERS

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I8ce47d13c53687d86303d14e3be52ea83ba2377c

Eliminate swampd core dump after D-Bus updates sensors

The swamp daemon intializes a list of sensors and uses those to
periodically scan the state associated devices. Reading the sensors is
done wit

Eliminate swampd core dump after D-Bus updates sensors

The swamp daemon intializes a list of sensors and uses those to
periodically scan the state associated devices. Reading the sensors is
done with an async timer, that runs code to re-arm an async timer.

There is also a D-Bus update cycle that is independent of the async
timer reading the sensors. When the D-Bus updates the number of
sensors in the system a new list must be created. In order to create
the new list the timers using the old list must be stopped. Only after
those timers have stopped may a new list be generated, and a new set of
timers started.

The two processes are unware of each other. To safely perform the
change the pointers to the list of zones and timers must be kept alive
until all timer actions complete. Only after all references to the
pointers have been release may the new state be built, and new timers
started.

Prior to this change swampd would throw a SYSSEGV fault due to an
attempt to use a pointer that was no longer active.

Tested:
Issued a "reset -w" (Warm Reset command) from the EFI shell.
Waited for the system to reboot, and enter EFI
Checked for a core file in /var/lib/systemd/coredump
Repeated step 1 if coredump file was not present.
Completed 2900+ passes successfully when ealier code failed at less
than 800 passes.

Change-Id: Iff4607510db579c36dc34d6f76e6eb2f0250a03a
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>

show more ...

bugfix: add virtual destructor to zoneinterface

ZoneInterface is a virtual abstract class and needs a virtual
destructor.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I4159a88327d

bugfix: add virtual destructor to zoneinterface

ZoneInterface is a virtual abstract class and needs a virtual
destructor.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I4159a88327d04bc344e484263f4c1d2875d81713

show more ...

dbusutil: Fix critical bug in scaleSensorReading

If minimum value is not 0,
as is common with temperature sensors of (-128,127) range,
the computed scaled value was wildly incorrect,
because there w

dbusutil: Fix critical bug in scaleSensorReading

If minimum value is not 0,
as is common with temperature sensors of (-128,127) range,
the computed scaled value was wildly incorrect,
because there was a missing step in the math.

Tested: Scaled values now appear correct to me

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I71290538168e17ac9ebb78489d359f012fbf8517

show more ...