summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* physical: Conform to LED class kernel ABIHEADmasterAndrew Jeffery2019-04-083-10/+14
| | | | | | | | | | | | | | | | | | | | 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>
* physical: Avoid unreachable statement in driveLED()Andrew Jeffery2019-04-081-6/+4
| | | | | | | This also slightly improves the branch coverage statistics Change-Id: I27d5168994831789a8f8dafeecc843242a7e406a Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* physical: Cleanup unnecessary variablesAndrew Jeffery2019-04-081-8/+3
| | | | | Change-Id: Ib5897e4f7226993c2deb1feb6e2e39d5a0f0cb5a Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* physical: Rework commentary for brevityAndrew Jeffery2019-04-081-42/+13
| | | | | Change-Id: I84341c6418853ad7fb44aa6f02ab298cc70842f6 Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* physical: 'frequency' is really periodicityAndrew Jeffery2019-04-082-11/+11
| | | | | Change-Id: I1007d2a7104fa07351c89c52a860cde83b2d509c Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* test: physical: Capture state(Action::{On,Off}) parametersAndrew Jeffery2019-04-081-2/+7
| | | | | | | | | 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>
* test: physical: Capture Action::Blink parametersAndrew Jeffery2019-04-081-1/+4
| | | | | | | | | 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>
* test: physical: Hit both branches of stableStateOperation()Andrew Jeffery2019-04-082-2/+20
| | | | | Change-Id: Ic32d87dbe996a03e811e1be88fbcc00bf450e1f3 Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* test: physical: Cover trigger:none, brigtness:asserted branchAndrew Jeffery2019-04-081-0/+10
| | | | | | | | | | | | | | | | | | | | 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>
* test: physical: Cover trigger:timer branch in setInitialState()Andrew Jeffery2019-04-081-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* test: physical: Introduce LED mocksAndrew Jeffery2019-04-081-5/+70
| | | | | | | | | | | | 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>
* sysfs: Integrate class into Physical and testsAndrew Jeffery2019-04-086-118/+41
| | | | | Change-Id: I7d5ad19df5ef1258a4e669ea3243b7411f371d9c Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* Add sysfs LED class wrapperAndrew Jeffery2019-03-184-1/+280
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* test: Add tests for Physical classAndrew Jeffery2019-03-184-2/+138
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* bootstrap: Add dev modeAndrew Jeffery2019-03-181-2/+25
| | | | | | | | | | 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>
* controller: Avoid pessimistic movesAndrew Jeffery2019-03-181-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Add OpenBMC C++ clang-format file and format codeAndrew Jeffery2019-03-186-198/+301
| | | | | Change-Id: Ib3a388bf5392159440682265b577fba023c3c3aa Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* phosphor-led-sysfs: use c++17Vernon Mauery2018-10-041-1/+1
| | | | | | | Update configure.ac to choose the c++17 standard Change-Id: I4dac04257a7b6758d28c8639d1ced9651cfaca90 Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>
* Add MAINTAINERS fileAndrew Jeffery2018-05-291-0/+47
| | | | | Change-Id: Ia82c8a9927202a045cc64c600c565d2de8d4c649 Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
* Add .gitignoreGunnar Mills2018-04-081-0/+46
| | | | | | | | 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 fixesGunnar Mills2018-04-081-1/+1
| | | | | | | | | 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>
* Work-around: Extract led name from device pathVishwanatha Subbanna2017-06-082-8/+21
| | | | | | | | | | | | | | | | | | | | | | 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>
* LEDS: Handle physical LED names containing hyphenVishwanatha Subbanna2017-03-311-0/+9
| | | | | | | | | | | | 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>
* Link to dbus interfaces library for using generated codeVishwanatha Subbanna2017-03-136-512/+10
| | | | | Change-Id: Id1c45d8c66deb499a92bfd94bc271f378245da34 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Check for needed package modules than individual header filesVishwanatha Subbanna2017-03-132-4/+3
| | | | | Change-Id: I123c143cd8ed65da3922f1928f96dc60de6e9458 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Add missing LT_INIT to configure.acVishwanatha Subbanna2017-02-201-0/+3
| | | | | Change-Id: Iba83a1fec052486f64a9d7a685bbc9298a5bd707 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Adhere to freedesktop naming convention for service and object pathVishwanatha Subbanna2017-02-201-4/+4
| | | | | | | Refer: https://dbus.freedesktop.org/doc/dbus-api-design.html Change-Id: Ic5d25d7c2a1b8e17c4fb286e73b6da62793a845c Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Add methods to trigger action on LEDVishwanatha Subbanna2016-12-193-14/+222
| | | | | | | Implements the ON, OFF and BLINK operations on a given LED. Change-Id: I74b5ac01d8e76961999a2673d52d73296f5603d7 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Add overriding methods for the base interface definitionVishwanatha Subbanna2016-12-195-19/+133
| | | | | | | | Defines the functions that override the default setter for the led state property. Change-Id: Ic3a8d43cc783003c43f53df8f7e90d7fc4d6715a Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Use generated bindings for physical led controllerVishwanatha Subbanna2016-12-125-2/+560
| | | | | | | | Extends the generated interface and provides skeleton implementation of led controller. Change-Id: I13485f39755213b8a7324e526d6335f3ee0a2b67 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Add initial files for physical LED controllerVishwanatha Subbanna2016-12-066-0/+252
| | | | | | | | | This daemon uses sysfs entry for a particular LED and then triggers necessary action. Name and Path of the LED is needed as command line options to make sure this is genreric implementation. Change-Id: I3d52f1491fcb6ae8119b399a4d2c6770a3f5db30 Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
* Initial commitPatrick Williams2016-11-291-0/+201
OpenPOWER on IntegriCloud