Generalize VE.Direct to SolarCharger #1442
Conversation
include/VictronMppt.h
Outdated
| uint32_t getDataAgeMillis() const; | ||
| uint32_t getDataAgeMillis(size_t idx) const; | ||
| uint32_t getDataAgeMillis() final; | ||
| uint32_t getDataAgeMillis(size_t idx) final; |
There was a problem hiding this comment.
You removed the const from all these functions. I guess that is an oversight? final does not imply const. Please make all functions const in the interface that were const before. This diff should only add the "final" keyword.
There was a problem hiding this comment.
The respective functions should be const in SolarChargerClass as well.
include/SolarCharger.h
Outdated
| // TODO(andreasboehm): below methods are taken from VictronMppt to start abstracting | ||
| // solar chargers without breaking everything. | ||
| size_t controllerAmount(); | ||
| uint32_t getDataAgeMillis(); | ||
| uint32_t getDataAgeMillis(size_t idx); | ||
|
|
||
| // total output of all MPPT charge controllers in Watts | ||
| int32_t getPowerOutputWatts(); | ||
|
|
||
| // total panel input power of all MPPT charge controllers in Watts | ||
| int32_t getPanelPowerWatts(); | ||
|
|
||
| // sum of total yield of all MPPT charge controllers in kWh | ||
| float getYieldTotal(); | ||
|
|
||
| // sum of today's yield of all MPPT charge controllers in kWh | ||
| float getYieldDay(); | ||
|
|
||
| // minimum of all MPPT charge controllers' output voltages in V | ||
| float getOutputVoltage(); | ||
|
|
||
| std::optional<VeDirectMpptController::data_t> getData(size_t idx = 0); | ||
|
|
||
| bool isDataValid(); |
There was a problem hiding this comment.
I guess, for starters, we should only have these in the interface, and hand out a (const?) reference to the current provider. Once we know the functions that will actually be available in SolarChargerClass, we'll make them available here directly. Example: Yield values will not be implemented by the MQTT provider (in my opinion), so the respective getters will not be available. But getPanelPowerWatts() and getPowerOutputWatts() will be, but one std::optional<>, I guess? We'll have to figure it out later.
There was a problem hiding this comment.
I am currently working on figuring out which values will be general values (e.g. panelPowerWatts) and which are implementation specific (e.g. yieldTotal).
I planned to hand out a ref to the current provider to allow the vedirect mqtt handling to keep working as it its right now and provide 'stats' like we do for the batteries for all other use-cases.
There was a problem hiding this comment.
where will we be doing MQTT publishing and HASS auto-discovery? in the battery context, that's done by the stats, isn't it?
There was a problem hiding this comment.
That is true. The issue that i see is that the MQTT topics are tied to 'vedirect' because they also include that name. But we could deprecate them and provide replacements.
AndreasBoehm
force-pushed
the
rename-vedirect-to-solarcharger
branch
3 times, most recently
from
dcd578e to
cad70b1
Compare
Because this view shows the same information as the configuration I figured that we can remove it and avoid the hassle of refactoring it.
AndreasBoehm
force-pushed
the
rename-vedirect-to-solarcharger
branch
from
cad70b1 to
8c0ecbc
Compare
AndreasBoehm
force-pushed
the
rename-vedirect-to-solarcharger
branch
from
8c0ecbc to
afcbc95
Compare
AndreasBoehm
force-pushed
the
rename-vedirect-to-solarcharger
branch
from
f61f456 to
f43b7a5
Compare
This is @AndreasBoehm's work to start generalizing the VE.Direct implementation into an abstract solar charger, which allows to later add a different provider/implementation (MQTT) retrieving info related to the solar charger in a system.