Conversation
While we stick to Apache-2.0 license, let's use a custom license header in our files to emphasize our work. While at, unify all license URLs to be in sync with want EVerest upstream uses. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
This will and can be used a helper library in 3rd party projects. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
This new helper function acquires a GPIO line by name and reports the current state back as bool. This can be used for one-time decisions but should not be used when the GPIO is expected to change later during runtime and/or the GPIO should be hold (and thus locked). Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
On Tarragon platform, there is a rotary encoder which can be used by customers to select the maximum current and/or the count of phases of the grid connection. This part here allows to use the 'MSB' line of this rotary encoder or any other GPIO as indication whether the charging station is wired with all three phases or only with a single phase to the grid. If no GPIO line name is configured, then it is assumed to be in 3 phase wiring. Additionally, when the configuration is suggesting that phase switching is supported, but this GPIO line indicates that only a single phase is available, then this is logged and phases switching support is not announced anymore. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
This can be used on (AC) carrier boards to select the count of phases of the grid connection, e.g. with a DIP switch. If no GPIO line name is configured, then the values from configuration file are used. Additionally, when the configuration is suggesting that phase switching is supported, but this GPIO line indicates that only a single phase is available, then this is logged and phases switching support is not announced anymore. Important note: CbChargeSOMDriver is not yet well-prepared for AC use-cases, for example the phase count switching infrastructure is still missing. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
Now that our BSP drivers gained support for grid phase count evaluation via GPIO (which is the cleaner approach since the BSP then only announces the available phase count), we can rework this module to propagate only the current limits, but without any phase restrictions. The GPIO line with the MSB (of the DIP switch or the rotary encoder) should now be configured in the corresponding BSP module. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
The used GPIO line could be of interest by multiple processes and when it happens that the line is busy, just retry after a short waiting. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
These functions might be of interest for future modules, too, so let's factor them out into our library. Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
barsnick
left a comment
barsnick
left a comment
There was a problem hiding this comment.
I see a lot of room for member functions, intermediate variables and constants becoming const or constexpr, but that's just style. Looks okay otherwise, but see separate remarks.
For the record, I get this on my DavyBox (Charge Control C):
[INFO] gpiolimits:CbGP :: Limiting to 32 A/phase, w/o max phase count
[INFO] bsp:CbTarragonD :: 1-phase wiring mode
(the latter line only if I actually configure the GPIO).
| is connected with three or one phase to the grid. When defined, then the GPIO should | ||
| be 'active' to indicate that is is connected with three phases. When the GPIO | ||
| signals only a single phase wiring, then features like phase count switching are | ||
| disabled. |
There was a problem hiding this comment.
Since the CbGPIOEnergyLimits already assumes hard-coded defaults, can we not set ROTARY_SWITCH_1_8_N as the default here, or at least hint in the description that this would be the one to use in case of rotary encoder?
There was a problem hiding this comment.
Those are two different things. I don't want to set a default because I don't want to push customers in the direction to bother with the rotary encoder at all. It was a nice idea, but the boards can be also used without using the rotary encoder switch at all. But I like the idea to add the line name to the description, will extend it.
| signals only a single phase wiring, then features like phase count switching are | ||
| disabled. | ||
| type: string | ||
| default: "" |
There was a problem hiding this comment.
Since the CbGPIOEnergyLimits already assumes hard-coded defaults, can we not set CURRENT_SETTING_1 as the default here, or at least hint in the description that this would be the one to use in case of DIP switch?
There was a problem hiding this comment.
This is even more difficult: we cannot set a default value here because we don't know on with carrier board we operate and whether it has support for such a feature. And I dont want to add different hints for different carrier boards here, so I'd leave it to customers to get it right.
lib: address review comments Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
CbGPIOEnergyLimits: address review comments Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
…a GPIO CbTarragonDriver: address review comments Signed-off-by: Michael Heimpold <michael.heimpold@chargebyte.com>
barsnick
left a comment
barsnick
left a comment
There was a problem hiding this comment.
LGTM so far - will approve when ready.
2 participants
This PR introduces a new module which is intended for apply energy limits based on GPIO inputs.
Such inputs can have different physical forms: on chargebyte's Charge Control C boards this is for example rotary encoder, on other boards, this could be a simple DIP switch bank with one or more switches.
While working on this topic, it was found that the GPIO helper stuff and other Linux low-level related funtions can be re-used in 3rd party projects. This is why part of this PR factors this functionality out into a new shared library.
However, it is difficult to move also the stuff which is tightly coupled to EVerest, so this part is left alone for the moment.