forked from tbnobody/OpenDTU
-
-
Notifications
You must be signed in to change notification settings - Fork 102
Enable full power range of battery-powered HM-xxx inverter on 24VDC #2372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
SW-Niko
wants to merge
26
commits into
hoylabs:development
Choose a base branch
from
SW-Niko:DPLuseATF
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
9e11925
Runtime: Base
SW-Niko fc84ebd
Runtime: Integration
SW-Niko 10c327f
Runtime: fix read() timing
SW-Niko 15de96f
Runtime: remove static bool from getWriteTrigger()
SW-Niko e2b03c4
Runtime: commit the new state only after a successful write
SW-Niko d54e11c
Runtime: move write() to onFirmwareUpdateFinish()
SW-Niko 2ce71c8
Runtime: move default value from .cpp to .h
SW-Niko 8982927
Runtime: use lock for protect _lastTrigger
SW-Niko 97ffc77
Runtime: enable write on demand, fix lock issue in write()
SW-Niko 9132262
Runtime: add missing bracket
SW-Niko 33bac66
BatteryState: refactor getBatteryPower()
SW-Niko 4581151
BatteryState: use the runtime file to store the state
SW-Niko 5218d11
BatteryState: remove debug logging
SW-Niko 00f0b79
PowerLimiter: add AllowStandby
SW-Niko b116ab9
InverterMeter: configuration
SW-Niko 05a46c0
InverterMeter: base Up1
SW-Niko 9007e53
InverterMeter: integration Up1
SW-Niko 1ccd9ad
InverterMeter: webApi
SW-Niko 7c1be3d
InverterMeter: webUI
SW-Niko 934dbb4
Runtime: Base (added read mode ON_DEMAND)
SW-Niko 0b74992
InverterATF: Configuration
SW-Niko 8434f0f
InverterATF: Base Up1-24
SW-Niko 70bd759
InverterATF: Integration Up1-11
SW-Niko 9a98b02
InverterATF: WebApp Up1-4
SW-Niko 7d996ef
yarn prettier
SW-Niko d2f1f5d
InverterATF: Base (solve cpplint issue)
SW-Niko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||
|
|
||
| /* | ||
| * Locking policy: | ||
| * - Public getters having the prefix 'get' take a shared lock internally. | ||
| * - Public mutating methods take an exclusive lock internally. | ||
| * - Private getters having the prefix 'g' do not take a lock. | ||
| * - Private methods are called with the appropriate lock held by the public methods. | ||
| * - Fetching external data is done before acquiring _mutex. The only exception is data from the configuration or | ||
| * data used just for visualization. | ||
| * - Flags marked as atomic may be read lock-free. | ||
| */ | ||
|
|
||
| #pragma once | ||
| #include <Arduino.h> | ||
| #include <shared_mutex> | ||
| #include <atomic> | ||
| #include <ArduinoJson.h> | ||
|
|
||
|
|
||
| class InverterATF { | ||
| public: | ||
| InverterATF() = default; | ||
| ~InverterATF() = default; | ||
| InverterATF(const InverterATF&) = delete; | ||
| InverterATF& operator=(const InverterATF&) = delete; | ||
| InverterATF(InverterATF&&) = delete; | ||
| InverterATF& operator=(InverterATF&&) = delete; | ||
|
|
||
|
|
||
| // indicates whether ATF is actually active | ||
| bool isATFActive() const { return _useATF.load(); } | ||
|
|
||
| // activate the ATF data structure | ||
| // use a unique lock when calling this method | ||
| bool activateATF(uint16_t const nomPower); | ||
|
|
||
| // deactivate the ATF data structure | ||
| // use a unique lock when calling this method | ||
| void deactivateATF(void); | ||
|
|
||
| // update the ATF data with the current power and limit pair | ||
| // use a unique lock when calling this method | ||
| void setATFData(float const power, float const limit); | ||
|
|
||
| // deserialize the ATF data from the runtime data file | ||
| // use a unique lock when calling this method | ||
| void deserializeATFData(JsonObject obj); | ||
|
|
||
| // serialize the ATF data to the runtime data file | ||
| // use a shared lock when calling this method | ||
| void serializeATFData(JsonObject obj) const; | ||
|
|
||
| // print the ATF report to the log | ||
| // use a shared lock when calling this method | ||
| void printATFReport(char const* serialStr) const; | ||
|
|
||
| // returns the power for the given limit according to the ATF | ||
| // use a shared/unique lock when calling this method | ||
| uint16_t getATFPower(float const limit) const; | ||
|
|
||
| // returns the limit for the given power according to the ATF | ||
| // use a shared/unique lock when calling this method | ||
| float getATFLimit(uint16_t const power) const; | ||
|
|
||
| private: | ||
| float makeAveragePower(float newValue, float const oldValue); | ||
| enum class State : uint8_t { OFF, DEFAULT_INIT, RTD_INIT }; | ||
|
|
||
| std::atomic<bool> _useATF = false; // false: ATF inactive and no memory allocated for the data array | ||
| State _state = State::OFF; // state of the ATF data array | ||
| static constexpr uint8_t _size = 101; // Fixed size of the ATF data array, never NEVER change the size! | ||
| std::unique_ptr<float[]> _realPower = nullptr; // ATF data array, index 0-100% | ||
|
|
||
| uint16_t _nomInvPower = 0; // Inverter nominal power [W] | ||
| uint16_t _maxInvPower = 0; // Inverter maximum power (nominal power * MAX_OVERDRIVE_FACTOR) [W] | ||
| uint16_t _absDiffPower = 0; // maximum absolute difference, used for power value checks [W] | ||
| mutable std::pair<uint16_t, float> _cache; // cache to avoid recalculations, first = power, second = limit | ||
| mutable std::shared_mutex _mutex; // mutex to protect the shared data | ||
|
|
||
| uint16_t _linearFault = 0; // counts the number of range check faults | ||
| uint16_t _averageWarning = 0; // counts the number of average warnings | ||
| std::pair<uint16_t, float> _linearPairFault; // buffer the maximum range check fault | ||
| std::pair<float, float> _averagePairWarning; // buffer the maximum average check warning | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||
| #pragma once | ||
|
|
||
| #include <ArduinoJson.h> | ||
| #include <TaskSchedulerDeclarations.h> | ||
| #include <mutex> | ||
| #include <atomic> | ||
|
|
||
|
|
||
| class RuntimeClass { | ||
| public: | ||
| explicit RuntimeClass(uint16_t version) : _writeVersion(version) {}; | ||
| ~RuntimeClass() = default; | ||
| RuntimeClass(const RuntimeClass&) = delete; | ||
| RuntimeClass& operator=(const RuntimeClass&) = delete; | ||
| RuntimeClass(RuntimeClass&&) = delete; | ||
| RuntimeClass& operator=(RuntimeClass&&) = delete; | ||
|
|
||
| void init(Scheduler& scheduler); | ||
| enum class ReadMode : uint8_t { START_UP, ON_DEMAND }; | ||
| bool read(ReadMode const mode = ReadMode::START_UP); // read runtime data | ||
| bool write(uint16_t const freezeMinutes = 10); // do not write if last write operation was less than freezeMinutes ago | ||
| void requestWriteOnNextTaskLoop(void) { _writeNow = true; }; // use this member function to store data on demand | ||
| void requestReadOnNextTaskLoop(void) { _readNow = true; }; // use this member function to read data on demand | ||
|
|
||
| uint16_t getWriteCount(void) const; | ||
| time_t getWriteEpochTime(void) const; | ||
| bool getReadState(void) const { return _readOK; } | ||
| bool getWriteState(void) const { return _writeOK; } | ||
| String getWriteCountAndTimeString(void) const; | ||
|
|
||
| private: | ||
| void loop(void); | ||
| bool getWriteTrigger(void); | ||
|
|
||
| Task _loopTask; | ||
| std::atomic<bool> _readOK = false; // true if the last read operation was successful | ||
| std::atomic<bool> _writeOK = false; // true if the last write operation was successful | ||
| std::atomic<bool> _readNow = false; // if true, the data is read in the next loop() | ||
| std::atomic<bool> _writeNow = false; // if true, the data is stored in the next loop() | ||
| mutable std::mutex _mutex; // to protect the shared data below | ||
| bool _lastTrigger = false; // auxiliary value to prevent multiple triggering on the same day | ||
| uint16_t _writeVersion = 0; // shared data: version of the runtime data | ||
| uint16_t _writeCount = 0; // shared data: number of write operations | ||
| time_t _writeEpoch = 0; // shared data: epoch time when the data was written | ||
| }; | ||
|
|
||
| extern RuntimeClass RuntimeData; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||
| #pragma once | ||
|
|
||
| #include <ESPAsyncWebServer.h> | ||
| #include <TaskSchedulerDeclarations.h> | ||
| #include <ArduinoJson.h> | ||
| #include "Configuration.h" | ||
|
|
||
| class WebApiInverterMeterClass { | ||
| public: | ||
| void init(AsyncWebServer& server, Scheduler& scheduler); | ||
|
|
||
| private: | ||
| void onStatus(AsyncWebServerRequest* request); | ||
| void onAdminGet(AsyncWebServerRequest* request); | ||
| void onAdminPost(AsyncWebServerRequest* request); | ||
| void onTestHttpJsonRequest(AsyncWebServerRequest* request); | ||
| void onTestHttpSmlRequest(AsyncWebServerRequest* request); | ||
|
|
||
| AsyncWebServer* _server; | ||
| }; | ||
|
SW-Niko marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||
| #pragma once | ||
|
|
||
| /* | ||
| * Locking policy: | ||
| * - Public getters having the prefix 'get' take a shared lock internally. | ||
| * - Public mutating methods take an exclusive lock internally. | ||
| * - Private getters having the prefix 'g' do not take a lock. | ||
| * - Private methods are called with the appropriate lock held by the public methods. | ||
| * - Fetching external data is done before acquiring _mutex. The only exception is data from the configuration or | ||
| * data used just for visualization. | ||
| * - Flags marked as atomic may be read lock-free. | ||
| */ | ||
|
|
||
| #include <powermeter/Provider.h> | ||
| #include <TaskSchedulerDeclarations.h> | ||
| #include <memory> | ||
| #include <mutex> | ||
|
|
||
|
SW-Niko marked this conversation as resolved.
|
||
| namespace InverterMeters { | ||
|
|
||
| class Controller { | ||
| public: | ||
| void init(Scheduler& scheduler); | ||
| void updateSettings(); | ||
|
|
||
| // todo: consider removing getPowerTotal in favor of getPower with inverterID | ||
| float getPowerTotal() const; | ||
| uint32_t getLastUpdate() const; | ||
|
|
||
| // returns true if the data is not older than 30 seconds | ||
| bool isDataValid() const; | ||
|
|
||
| // returns power for the given inverter ID if available | ||
| std::optional<float> getPower(uint64_t inverterID) const; | ||
|
|
||
| // returns the time of the last update for the given inverter serial number | ||
| uint32_t getTime(uint64_t inverterSN) const; | ||
|
|
||
| // returns the time of the measurement request in milliseconds | ||
| uint32_t getRequestTime() const; | ||
|
|
||
| private: | ||
| void loop(); | ||
|
|
||
| Task _loopTask; | ||
| mutable std::mutex _mutex; | ||
| std::unique_ptr<PowerMeters::Provider> _upProvider = nullptr; | ||
| }; | ||
|
|
||
| } // namespace InverterMeters | ||
|
|
||
| extern InverterMeters::Controller InverterMeter; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a virtual destructor and clean up ATF helper surface
The new ATF integration and external meter plumbing are conceptually fine, but there are a couple of structural issues:
Non-virtual base destructor with polymorphic use
PowerLimiterInverteris used polymorphically (std::unique_ptr<PowerLimiterInverter>holding derived instances), but it has no virtual destructor. This is undefined behavior when deleting derived objects via a base pointer. Please add:Inline ATF helpers couple the header tightly to implementation details
setATFData()calls intoInverterATF::setATFData(getCurrentOutputAcWatts(), _spInverter->SystemConfigPara()->getLimitPercent());directly from the header. That’s OK, but it makes every inclusion of this header depend on these internals; consider moving the definition to the.cppif compile times become an issue.getATFConfigPower()is marked “todo: delete after testing”. It’s better to remove or#ifdeftest-only APIs before merging to avoid public API creep.External meter state
_oInverterMeterPowerasstd::optional<float>aligns with the new override semantics; just ensure any concurrent access is guarded consistently with how the rest of the class is used.Also applies to: 78-99, 162-164