Add ability to set LED colors in DBus

phosphor-led-sysfs parses sysfs LED node names
in the form of "devicename:colour:function" as
described in

https://github.com/torvalds/linux/blob/master/Docume

Add ability to set LED colors in DBus

phosphor-led-sysfs parses sysfs LED node names
in the form of "devicename:colour:function" as
described in

https://github.com/torvalds/linux/blob/master/Documentation/leds/leds-class.txt#L46

and then converts the "colour" part into the proper
value of the DBus 'Color' property using the
xyz.openbmc_project.Led.Physical.Palette interface.

In order for the led nodes to have their Color set,
the corresponding led nodes in devicetree must have
correct 'label' properties, e.g.:

leds {
compatible = "gpio-leds";

heartbeat {
label = "bmc:green:hearbeat";
gpios = <&gpio ASPEED_GPIO(R, 4) GPIO_ACTIVE_LOW>;
};
power_red {
label = "system:red:power";
gpios = <&gpio ASPEED_GPIO(N, 1) GPIO_ACTIVE_LOW>;
};
}

Change-Id: I094f0e434bdd262995752576a3c792ccd5dbb3e2
Signed-off-by: Alexander Soldatov <a.soldatov@yadro.com>
Signed-off-by: Alexander Amelkin <a.amelkin@yadro.com>

show more ...

physical: Conform to LED class kernel ABI

The kernel says the following about the LED sysfs interface:

> LED handling under Linux
> ========================
>
> In its simplest form, the LED class

physical: Conform to LED class kernel ABI

The kernel says the following about the LED sysfs interface:

> LED handling under Linux
> ========================
>
> In its simplest form, the LED class just allows control of LEDs from
> userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
> LED is defined in max_brightness file. The brightness file will set the brightness
> of the LED (taking a value 0-max_brightness). Most LEDs don't have hardware
> brightness support so will just be turned on for non-zero brightness settings.

The existing code assumed that max_brightness always held a value of
255 and defined a constant for it. Instead, use a class variable to
cache the max brightness for the associated LED.

Change-Id: I2d8f46de0cddac5f9d8ff5444449518bb4056130
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

physical: Avoid unreachable statement in driveLED()

This also slightly improves the branch coverage statistics

Change-Id: I27d5168994831789a8f8dafeecc843242a7e406a
Signed-off-by: Andrew Jeffery <an

physical: Avoid unreachable statement in driveLED()

This also slightly improves the branch coverage statistics

Change-Id: I27d5168994831789a8f8dafeecc843242a7e406a
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

physical: Cleanup unnecessary variables

Change-Id: Ib5897e4f7226993c2deb1feb6e2e39d5a0f0cb5a
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

physical: Rework commentary for brevity

Change-Id: I84341c6418853ad7fb44aa6f02ab298cc70842f6
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

physical: 'frequency' is really periodicity

Change-Id: I1007d2a7104fa07351c89c52a860cde83b2d509c
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

test: physical: Capture state(Action::{On,Off}) parameters

Implement value sensing to verify the behaviour of the On and Off
actions. This constrains future on/off changes to interacting with sysfs

test: physical: Capture state(Action::{On,Off}) parameters

Implement value sensing to verify the behaviour of the On and Off
actions. This constrains future on/off changes to interacting with sysfs
in the same manner.

Change-Id: Ie0ac063cbda3156ca7f0ef2eab9e700013780e44
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

test: physical: Capture Action::Blink parameters

Implement value sensing to verify behaviour of the blink action. This
constrains future blink changes to interacting with sysfs in the same
manner.

test: physical: Capture Action::Blink parameters

Implement value sensing to verify behaviour of the blink action. This
constrains future blink changes to interacting with sysfs in the same
manner.

Change-Id: I7ed0a5d52e8dc99192e7938760ce99f618b9cb16
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

test: physical: Hit both branches of stableStateOperation()

Change-Id: Ic32d87dbe996a03e811e1be88fbcc00bf450e1f3
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

test: physical: Cover trigger:none, brigtness:asserted branch

The test causes execution to take the following path in
setInitialState(), previously outlined in 264d909d3dc9 ("test: Add tests
for Phy

test: physical: Cover trigger:none, brigtness:asserted branch

The test causes execution to take the following path in
setInitialState(), previously outlined in 264d909d3dc9 ("test: Add tests
for Physical class") as being missed:

auto brightness = led.getBrightness();
if (brightness == ASSERT)
{
// LED is in Solid ON
sdbusplus::xyz::openbmc_project::Led::server ::Physical::state(
Action::On);
}

This brings the line coverage to 97.6% (function coverage remains
unchanged by this patch).

Change-Id: I7a04fcd97819559575e69d267c62f16195495010
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

test: physical: Cover trigger:timer branch in setInitialState()

The test causes execution to take the following path in
setInitialState(), previously outlined in 264d909d3dc9 ("test: Add tests
for P

test: physical: Cover trigger:timer branch in setInitialState()

The test causes execution to take the following path in
setInitialState(), previously outlined in 264d909d3dc9 ("test: Add tests
for Physical class") as being missed:

auto trigger = led.getTrigger();
if (trigger == "timer")
{
// LED is blinking. Get the delay_on and delay_off and compute
// DutyCycle. sfsfs values are in strings. Need to convert 'em over to
// integer.
auto delayOn = led.getDelayOn();
auto delayOff = led.getDelayOff();

// Calculate frequency and then percentage ON
frequency = delayOn + delayOff;
auto factor = frequency / 100;
auto dutyOn = delayOn / factor;

// Update.
this->dutyOn(dutyOn);
}

This brings the line coverage to 96.6% (function coverage remains
unchanged by this patch).

Change-Id: Ie311186f6275d3fdd31de5698c7c14bb335128e1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

test: physical: Introduce LED mocks

This removes the dependency on touching the filesystem entirely. All
methods are now mocked into an ignored state when called by the NiceMock
template class. The

test: physical: Introduce LED mocks

This removes the dependency on touching the filesystem entirely. All
methods are now mocked into an ignored state when called by the NiceMock
template class. The filesystem is only touched by the SysfsLed tests,
though we still need to provide a temporary path to its constructor in
the decended mock class to ensure we're isolated if something does
manage to get written.

Change-Id: I3955a6e0fb5c3c42887da847239d381ef151fa3e
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

sysfs: Integrate class into Physical and tests

Change-Id: I7d5ad19df5ef1258a4e669ea3243b7411f371d9c
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Add sysfs LED class wrapper

The Physical class implements private templated read() and write()
methods. There are several properties that make this approach less than
ideal:

1. read() and write() a

Add sysfs LED class wrapper

The Physical class implements private templated read() and write()
methods. There are several properties that make this approach less than
ideal:

1. read() and write() are non-virtual template functions, and we would
have to link-seam mock the internals, which means hijacking
std::fstream.

2. Even if read() and write() were virtual functions, this is made
irrelevant by the fact that we want to traverse execution paths in
setInitialState(), which is called from the Physical constructor.
Methods invoked by class constructors are bound to the implementations
specified in the constructor's class and will never dispatch to a
descendant's override. As such we have no mechanism to manipulate the
execution path without resorting to the pre-processor seam, which is
undesirable for other reasons.

3. The abstraction is poor. Physical implements the business logic of
converting interactions with the DBus interface into compatible
interactions with the sysfs LED class attributes, and shouldn't have
knowledge of how to directly interact with sysfs itself.

4. read() and write() are template functions, but the only type
parameter used in the code-base is std::string, and conversions are
left to the caller. This needlessly complicates the caller logic and
reduces readability of the callee code.

The change defines a separate class, SysfsLed, to map actions onto the
LED sysfs class attributes. SysfsLed will be provided by the
dependency-injection pattern to the Physical class by passing an
instance reference through its constructor. The lifetime of the SysfsLed
instance must exceed the lifetime of the associated Physical instance.

Further, the methods of SysfsLed are all marked as virtual and defined
to return concrete types (either unsigned long or std::string as
appropriate). This opens the door for mocking without resorting to
techniques such as using link seams, and removes templates as a point of
complication. Further, defining only a concrete class and not an
abstract base class minimises the boilerplate required as we're likely
never going to have another descendant of SysfsLed that isn't a mock
implementation (we don't need to exclude implementations by way of
sibling types).

Integration tests are provided for SysfsLed, which is necessary as the
class must write to the filesystem (again, unless we want to hijack
std::fstream, which seems unpalatable). Isolated temporary directories
are used to ensure the tests can be run in parallel without
interference. The tests provide 100% line coverage of SysfsLed.

Change-Id: I81fc7d9fd07eed54035f515502f563f25aa1e58f
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

test: Add tests for Physical class

The additions to configure expose a new `check-code-coverage` make
target when `--enable-code-coverage` is passed to `./configure`.
Assuming gcov/lcov are installe

test: Add tests for Physical class

The additions to configure expose a new `check-code-coverage` make
target when `--enable-code-coverage` is passed to `./configure`.
Assuming gcov/lcov are installed, `make check-code-coverage` will run
the test suite and generate an HTML line/function/branch coverage report
that enables measurement of the effectiveness of the test suite.

The tests themselves are trivial (integration) tests that get us to
78.8% line coverage and 93.3% function coverage over physical.hpp and
physical.cpp.

However, as we don't have the read() and write() functions under our
control - and as they're implemented to return empty strings when the
target files do not exist - this high level of coverage is more by luck
than design.

To demonstrate, under the current test arrangement, we can never enter
this branch of setInitialState():

auto trigger = read<std::string>(blinkCtrl);
if (trigger == "timer")
{
// LED is blinking. Get the delay_on and delay_off and compute
// DutyCycle. sfsfs values are in strings. Need to convert 'em over to
// integer.
auto delayOn = std::stoi(read<std::string>(delayOnCtrl));
auto delayOff = std::stoi(read<std::string>(delayOffCtrl));

// Calculate frequency and then percentage ON
frequency = delayOn + delayOff;
auto factor = frequency / 100;
auto dutyOn = delayOn / factor;

// Update.
this->dutyOn(dutyOn);
}

For similar reasons, we also fail to enter:

auto brightness = read<std::string>(brightCtrl);
if (brightness == std::string(ASSERT))
{
// LED is in Solid ON
sdbusplus::xyz::openbmc_project::Led::server ::Physical::state(
Action::On);
}

To test both of these paths we need to make changes to isolate
functionality so we can manipulate the read() call to return the
necessary strings at the appropriate times, but that is for a future
change.

Change-Id: I0df2ab2d992ccad514cddb7f7fc6d080aa74f27d
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

bootstrap: Add dev mode

Running `./bootstrap.sh dev` enables various sanitisers in GCC and
code-coverage targets in the generated Makefile. `./configure` is run
automatically as part of dev mode, as

bootstrap: Add dev mode

Running `./bootstrap.sh dev` enables various sanitisers in GCC and
code-coverage targets in the generated Makefile. `./configure` is run
automatically as part of dev mode, as it is a shortcut for adding the
above options to the configure script invocation.

Change-Id: I48b9a312f438efd070ad5f982be80326894b1141
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

controller: Avoid pessimistic moves

clang-6.0 reports that some of the moves prevent further optimisations, so
remove them.

$ make
make all-am
make[1]: Entering directory '/home/andrew/src/openbmc

controller: Avoid pessimistic moves

clang-6.0 reports that some of the moves prevent further optimisations, so
remove them.

$ make
make all-am
make[1]: Entering directory '/home/andrew/src/openbmc/phosphor-led-sysfs'
CXX controller.o
controller.cpp:60:12: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
path = std::move(DEVPATH + name);
^
controller.cpp:60:12: note: remove std::move call here
path = std::move(DEVPATH + name);
^~~~~~~~~~ ~
controller.cpp:75:16: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
auto bus = std::move(sdbusplus::bus::new_default());
^
controller.cpp:75:16: note: remove std::move call here
auto bus = std::move(sdbusplus::bus::new_default());
^~~~~~~~~~ ~
2 errors generated.
Makefile:761: recipe for target 'controller.o' failed
make[1]: *** [controller.o] Error 1
make[1]: Leaving directory '/home/andrew/src/openbmc/phosphor-led-sysfs'
Makefile:617: recipe for target 'all' failed
make: *** [all] Error 2

Change-Id: I3da6415def4ab183cc9f6c5e176e8bb3666cf1b7
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

Add OpenBMC C++ clang-format file and format code

Change-Id: Ib3a388bf5392159440682265b577fba023c3c3aa
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

phosphor-led-sysfs: use c++17

Update configure.ac to choose the c++17 standard

Change-Id: I4dac04257a7b6758d28c8639d1ced9651cfaca90
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>

Add MAINTAINERS file

Change-Id: Ia82c8a9927202a045cc64c600c565d2de8d4c649
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Add .gitignore

Took a .gitignore from another repo and added a few repo
specific items.

Change-Id: I42e71250ec714181d09a4449695d2905ce456203
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

Spelling fixes

Spelling errors found using github.com/lucasdemarchi/codespell
A tool to fix common misspellings.
This tool is licensed under GNU General Public License, version 2.

Change-Id: I9f392

Spelling fixes

Spelling errors found using github.com/lucasdemarchi/codespell
A tool to fix common misspellings.
This tool is licensed under GNU General Public License, version 2.

Change-Id: I9f3925794f1a6318f1ec5172c2d10e18e9aedfa6
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Work-around: Extract led name from device path

udev rule for leds subsystem in Witherspoon launches a systemd
service file with /sys/class/leds/$name. If the path is
sys-class-leds-rear-fault, syste

Work-around: Extract led name from device path

udev rule for leds subsystem in Witherspoon launches a systemd
service file with /sys/class/leds/$name. If the path is
sys-class-leds-rear-fault, systemd service file
interprets it as /sys/class/leds/rear/fault.

However, what is really needed by the service file is
/sys/class/leds/rear-fault.

This is a limitation in current systemd with template argument
containing hyphen.

Short term solution is to extract $name from path
and convert "/" to "-". It would then become:
/sys/class/leds/rear-fault and hence will work.

Refer: systemd/systemd#5072

Change-Id: I0acc11d039650857005ba75810e3ef6bcc4a3934
Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>

show more ...

LEDS: Handle physical LED names containing hyphen

It is possible that LED names can contain hyphen(s) in them making it
inappropriate for announcing the name on dbus. Fix would be to
convert the hyp

LEDS: Handle physical LED names containing hyphen

It is possible that LED names can contain hyphen(s) in them making it
inappropriate for announcing the name on dbus. Fix would be to
convert the hyphen to underscores just for announcing on dbus
while using the actual name for rest everything.

Fixes openbmc/openbmc#802

Change-Id: Ia916786e9abf948970b36242f57c27aa90d4a962
Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>

show more ...

Link to dbus interfaces library for using generated code

Change-Id: Id1c45d8c66deb499a92bfd94bc271f378245da34
Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>