search

LibreSolar / charge-controller-firmware

Firmware for Libre Solar MPPT/PWM charge controllers
https://libre.solar/charge-controller-firmware/
Apache License 2.0
144 stars 71 forks source link

Merge state machines and error flags #91

Closed martinjaeger closed 4 years ago

martinjaeger commented 4 years ago

We are currently using one global error_flags variable stored in DeviceStatus class to report errors. This is an inflexible solution if we add more loads and if also the USB output becomes one instance of the LoadOutput class (currently working on that change) as

  1. we can't easily support different hardware configurations with less or more output switches and
  2. we might run out of flags (only 32 possible).

Suggested change: Use one int state variable in each subsystem (LoadOutput, Charger, PwmSwitch, Dcdc) which stores either the state for the state machine (positive value) or the error flag(s) of this particular sub-system as a negative value. A negative value means automatically that the subsystem is not in operational state anymore. In the code, this would be handled by the default switch case, meaning error state now. The initial state would be value 0, meaning the subsystem is in intialization or idle state. Positive means operational, but we can have different operational states, e.g. CC or CV charging, derating because limits were reached, etc..

Any arguments against this change or other ideas?

azeemshatp commented 4 years ago

Having a dedicated state variable per subsystem makes sense to me, which helps for scalability and reduce complexity. Let's go for it.

martinjaeger commented 4 years ago

During implementation I realized that it's probably not a good idea to merge state machine and error flags for all subsystems, as we can have situations where an error flag is set, but limited operation (in a state other than ERROR) is still possible.

Example: Battery temperature too low for charging, so the ERR_BAT_CHG_UNDERTEMP flag is set. But we can still discharge the battery.

Anyway I'll probably add an additional data object LoadInfo which implements above described idea with negative error codes, so that we don't have to publish load state and load error flags. However, internally we will use both separately. It's also much easier to use bit manipulation for unsigned variables than for signed ones.

martinjaeger commented 4 years ago

LoadInfo added in 651f66616bd8c1baca1f194012d00e4e69bc16c6.