Short proposal on phosphor-hwmon refactoring

Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I3996ffe0b99c547312d46d05a4e7ec058f7b4695
diff --git a/designs/phosphor-hwmon-refactoring.md b/designs/phosphor-hwmon-refactoring.md
new file mode 100644
index 0000000..7a13087
--- /dev/null
+++ b/designs/phosphor-hwmon-refactoring.md
@@ -0,0 +1,73 @@
+# Phosphor-hwmon refactoring
+
+Author: Kun Yi <kunyi!>
+
+Created: 2019/09/05
+
+## Problem Description
+
+Convoluted code in phosphor-hwmon is imparing our ability to add features
+and identify/fix bugs.
+
+## Background and References
+
+A few cases of smelly code or bad design decisions throughout the code:
+
+* Limited unit testing coverage. Currently the unit tests in the repo [1] mostly
+   test that the sensor can be constructed correctly, but there is no testing
+   of correct behavior on various sensor reading or failure conditions.
+*  Bloated mainloop. mainloop::read() sits at 146 lines [2] with complicated
+   conditions, macros, and try/catch blocks.
+*  Lack of abstraction and dependency injection. This makes the code hard to
+   test and hard to read. For example, the conditional at [3]
+   can be replaced by implementing different sensor::readValue() method for
+   different sensor types.
+
+[1](https://github.com/openbmc/phosphor-hwmon/tree/master/test)
+[2](https://github.com/openbmc/phosphor-hwmon/blob/94a04c4e9162800af7b2823cd52292e3aa189dc3/mainloop.cpp#L390)
+[3](https://github.com/openbmc/phosphor-hwmon/blob/94a04c4e9162800af7b2823cd52292e3aa189dc3/mainloop.cpp#L408)
+
+## Goals
+
+1. Improve unit test coverage
+2. Improve overall design using OOD principles
+3. Improve error handling and resiliency
+4. Improve configurability
+    *  By providing a common configuration class, it will be feasible to add
+       alternative configuration methods, e.g. JSON, while maintaining backward
+       compatibility with existing env files approach.
+
+## Proposed Design
+
+Rough breakdown of refactoring steps:
+
+1. Sensor configuration
+    *  Add a SensorConfig struct that does all std::getenv() calls in one place
+    *  DI: make the sensor struct take SensorConfig as dependency
+    *  Unit tests for SensorConfig
+    *  Add unit tests for sensor creation with various SensorConfigs
+2. Abstract sensors
+    *  Refine the sensor object interface and make it abstract
+    *  Define sensor types that inherit from the common interface
+    *  Add unit tests for sensor interface
+    *  DI: make the sensor map take sensor interface as dependency
+    *  Add a fake sensor that can exhibit controllable behavior
+3. Refactor sensor management logic
+    *  Refactor sensor map to allow easier lookup/insertion
+    *  Add unit tests for sensor map
+    *  DI: make all other functions take sensor map as dependency
+4. Abstract error handling
+    *  Add overridable error handler to sensor interface
+    *  Define different error handlers
+    *  Add unit tests for error handlers using the fake sensor
+
+## Alternatives Considered
+N/A.
+
+## Impacts
+The refactoring should have no external API impact. Performance impact should
+be profiled.
+
+## Testing
+One of the goals is to have better unit test coverage. Overall functionality
+to be tested through functional and regression tests.