Skip to content

Commit

Permalink
add and use configuration write guard
Browse files Browse the repository at this point in the history 
the configuration write guard is now required when the configuration
struct shall be mutated. the write guards locks multiple writers against
each other and also, more importantly, makes the writes synchronous to
the main loop. all code running in the main loop can now be sure that
(1) reads from the configuration struct are non-preemtive and (2) the
configuration struct as a whole is in a consistent state when reading
from it.

NOTE that acquiring a write guard from within the main loop's task will
immediately cause a deadlock and the watchdog will trigger a reset. if
writing from inside the main loop should ever become necessary, the
write guard must be updated to only lock the mutex but not wait for a
signal.
  • Loading branch information
@schlimmchen
schlimmchen committed Oct 22, 2024
1 parent dc5eb96 commit b1edb13
Show file tree
Hide file tree
Showing 11 changed files with 243 additions and 149 deletions.
24 changes: 22 additions & 2 deletions include/Configuration.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

#include "PinMapping.h"
#include <cstdint>
#include <TaskSchedulerDeclarations.h>
#include <mutex>
#include <condition_variable>

#define CONFIG_FILENAME "/config.json"
#define CONFIG_VERSION 0x00011c00 // 0.1.28 // make sure to clean all after change
Expand Down Expand Up @@ -161,15 +164,32 @@ struct CONFIG_T {

class ConfigurationClass {
public:
void init();
void init(Scheduler& scheduler);
bool read();
bool write();
void migrate();
CONFIG_T& get();
CONFIG_T const& get();

class WriteGuard {
public:
WriteGuard();
CONFIG_T& getConfig();
~WriteGuard();

private:
std::unique_lock<std::mutex> _lock;
};

WriteGuard getWriteGuard();

INVERTER_CONFIG_T* getFreeInverterSlot();
INVERTER_CONFIG_T* getInverterConfig(const uint64_t serial);
void deleteInverterById(const uint8_t id);

private:
void loop();

Task _loopTask;
};

extern ConfigurationClass Configuration;
58 changes: 56 additions & 2 deletions src/Configuration.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,17 @@

CONFIG_T config;

void ConfigurationClass::init()
static std::condition_variable sWriterCv;
static std::mutex sWriterMutex;
static unsigned sWriterCount = 0;

void ConfigurationClass::init(Scheduler& scheduler)
{
scheduler.addTask(_loopTask);
_loopTask.setCallback(std::bind(&ConfigurationClass::loop, this));
_loopTask.setIterations(TASK_FOREVER);
_loopTask.enable();

memset(&config, 0x0, sizeof(config));
}

Expand Down Expand Up @@ -318,6 +327,20 @@ bool ConfigurationClass::read()
}

f.close();

// Check for default DTU serial
MessageOutput.print("Check for default DTU serial... ");
if (config.Dtu.Serial == DTU_SERIAL) {
MessageOutput.print("generate serial based on ESP chip id: ");
const uint64_t dtuId = Utils::generateDtuSerial();
MessageOutput.printf("%0" PRIx32 "%08" PRIx32 "... ",
((uint32_t)((dtuId >> 32) & 0xFFFFFFFF)),
((uint32_t)(dtuId & 0xFFFFFFFF)));
config.Dtu.Serial = dtuId;
write();
}
MessageOutput.println("done");

return true;
}

Expand Down Expand Up @@ -390,11 +413,16 @@ void ConfigurationClass::migrate()
read();
}

CONFIG_T& ConfigurationClass::get()
CONFIG_T const& ConfigurationClass::get()
{
return config;
}

ConfigurationClass::WriteGuard ConfigurationClass::getWriteGuard()
{
return WriteGuard();
}

INVERTER_CONFIG_T* ConfigurationClass::getFreeInverterSlot()
{
for (uint8_t i = 0; i < INV_MAX_COUNT; i++) {
Expand Down Expand Up @@ -439,4 +467,30 @@ void ConfigurationClass::deleteInverterById(const uint8_t id)
}
}

void ConfigurationClass::loop()
{
std::unique_lock<std::mutex> lock(sWriterMutex);
if (sWriterCount == 0) { return; }

sWriterCv.notify_all();
sWriterCv.wait(lock, [] { return sWriterCount == 0; });
}

CONFIG_T& ConfigurationClass::WriteGuard::getConfig()
{
return config;
}

ConfigurationClass::WriteGuard::WriteGuard()
: _lock(sWriterMutex)
{
sWriterCount++;
sWriterCv.wait(_lock);
}

ConfigurationClass::WriteGuard::~WriteGuard() {
sWriterCount--;
if (sWriterCount == 0) { sWriterCv.notify_all(); }
}

ConfigurationClass Configuration;
4 changes: 2 additions & 2 deletions src/WebApi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void WebApiClass::reload()

bool WebApiClass::checkCredentials(AsyncWebServerRequest* request)
{
CONFIG_T& config = Configuration.get();
auto const& config = Configuration.get();
if (request->authenticate(AUTH_USERNAME, config.Security.Password)) {
return true;
}
Expand All @@ -65,7 +65,7 @@ bool WebApiClass::checkCredentials(AsyncWebServerRequest* request)

bool WebApiClass::checkCredentialsReadonly(AsyncWebServerRequest* request)
{
CONFIG_T& config = Configuration.get();
auto const& config = Configuration.get();
if (config.Security.AllowReadonly) {
return true;
} else {
Expand Down
35 changes: 20 additions & 15 deletions src/WebApi_device.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,23 +129,28 @@ void WebApiDeviceClass::onDeviceAdminPost(AsyncWebServerRequest* request)
return;
}

CONFIG_T& config = Configuration.get();
bool performRestart = root["curPin"]["name"].as<String>() != config.Dev_PinMapping;

strlcpy(config.Dev_PinMapping, root["curPin"]["name"].as<String>().c_str(), sizeof(config.Dev_PinMapping));
config.Display.Rotation = root["display"]["rotation"].as<uint8_t>();
config.Display.PowerSafe = root["display"]["power_safe"].as<bool>();
config.Display.ScreenSaver = root["display"]["screensaver"].as<bool>();
config.Display.Contrast = root["display"]["contrast"].as<uint8_t>();
config.Display.Language = root["display"]["language"].as<uint8_t>();
config.Display.Diagram.Duration = root["display"]["diagramduration"].as<uint32_t>();
config.Display.Diagram.Mode = root["display"]["diagrammode"].as<DiagramMode_t>();

for (uint8_t i = 0; i < PINMAPPING_LED_COUNT; i++) {
config.Led_Single[i].Brightness = root["led"][i]["brightness"].as<uint8_t>();
config.Led_Single[i].Brightness = min<uint8_t>(100, config.Led_Single[i].Brightness);
{
auto guard = Configuration.getWriteGuard();
auto& config = guard.getConfig();

strlcpy(config.Dev_PinMapping, root["curPin"]["name"].as<String>().c_str(), sizeof(config.Dev_PinMapping));
config.Display.Rotation = root["display"]["rotation"].as<uint8_t>();
config.Display.PowerSafe = root["display"]["power_safe"].as<bool>();
config.Display.ScreenSaver = root["display"]["screensaver"].as<bool>();
config.Display.Contrast = root["display"]["contrast"].as<uint8_t>();
config.Display.Language = root["display"]["language"].as<uint8_t>();
config.Display.Diagram.Duration = root["display"]["diagramduration"].as<uint32_t>();
config.Display.Diagram.Mode = root["display"]["diagrammode"].as<DiagramMode_t>();

for (uint8_t i = 0; i < PINMAPPING_LED_COUNT; i++) {
config.Led_Single[i].Brightness = root["led"][i]["brightness"].as<uint8_t>();
config.Led_Single[i].Brightness = min<uint8_t>(100, config.Led_Single[i].Brightness);
}
}

auto const& config = Configuration.get();
bool performRestart = root["curPin"]["name"].as<String>() != config.Dev_PinMapping;

Display.setDiagramMode(static_cast<DiagramMode_t>(config.Display.Diagram.Mode));
Display.setOrientation(config.Display.Rotation);
Display.enablePowerSafe = config.Display.PowerSafe;
Expand Down
20 changes: 11 additions & 9 deletions src/WebApi_dtu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void WebApiDtuClass::init(AsyncWebServer& server, Scheduler& scheduler)
void WebApiDtuClass::applyDataTaskCb()
{
// Execute stuff in main thread to avoid busy SPI bus
CONFIG_T& config = Configuration.get();
auto const& config = Configuration.get();
Hoymiles.getRadioNrf()->setPALevel((rf24_pa_dbm_e)config.Dtu.Nrf.PaLevel);
Hoymiles.getRadioCmt()->setPALevel(config.Dtu.Cmt.PaLevel);
Hoymiles.getRadioNrf()->setDtuSerial(config.Dtu.Serial);
Expand Down Expand Up @@ -153,14 +153,16 @@ void WebApiDtuClass::onDtuAdminPost(AsyncWebServerRequest* request)
return;
}

CONFIG_T& config = Configuration.get();

config.Dtu.Serial = serial;
config.Dtu.PollInterval = root["pollinterval"].as<uint32_t>();
config.Dtu.Nrf.PaLevel = root["nrf_palevel"].as<uint8_t>();
config.Dtu.Cmt.PaLevel = root["cmt_palevel"].as<int8_t>();
config.Dtu.Cmt.Frequency = root["cmt_frequency"].as<uint32_t>();
config.Dtu.Cmt.CountryMode = root["cmt_country"].as<CountryModeId_t>();
{
auto guard = Configuration.getWriteGuard();
auto& config = guard.getConfig();
config.Dtu.Serial = serial;
config.Dtu.PollInterval = root["pollinterval"].as<uint32_t>();
config.Dtu.Nrf.PaLevel = root["nrf_palevel"].as<uint8_t>();
config.Dtu.Cmt.PaLevel = root["cmt_palevel"].as<int8_t>();
config.Dtu.Cmt.Frequency = root["cmt_frequency"].as<uint32_t>();
config.Dtu.Cmt.CountryMode = root["cmt_country"].as<CountryModeId_t>();
}

WebApi.writeConfig(retMsg);

Expand Down
78 changes: 44 additions & 34 deletions src/WebApi_inverter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ void WebApiInverterClass::onInverterEdit(AsyncWebServerRequest* request)
}

// Interpret the string as a hex value and convert it to uint64_t
const uint64_t serial = strtoll(root["serial"].as<String>().c_str(), NULL, 16);
const uint64_t new_serial = strtoll(root["serial"].as<String>().c_str(), NULL, 16);

if (serial == 0) {
if (new_serial == 0) {
retMsg["message"] = "Serial must be a number > 0!";
retMsg["code"] = WebApiError::InverterSerialZero;
WebApi.sendJsonResponse(request, response, __FUNCTION__, __LINE__);
Expand All @@ -209,37 +209,42 @@ void WebApiInverterClass::onInverterEdit(AsyncWebServerRequest* request)
return;
}

INVERTER_CONFIG_T& inverter = Configuration.get().Inverter[root["id"].as<uint8_t>()];

uint64_t new_serial = serial;
uint64_t old_serial = inverter.Serial;

// Interpret the string as a hex value and convert it to uint64_t
inverter.Serial = new_serial;
strncpy(inverter.Name, root["name"].as<String>().c_str(), INV_MAX_NAME_STRLEN);

inverter.Poll_Enable = root["poll_enable"] | true;
inverter.Poll_Enable_Night = root["poll_enable_night"] | true;
inverter.Command_Enable = root["command_enable"] | true;
inverter.Command_Enable_Night = root["command_enable_night"] | true;
inverter.ReachableThreshold = root["reachable_threshold"] | REACHABLE_THRESHOLD;
inverter.ZeroRuntimeDataIfUnrechable = root["zero_runtime"] | false;
inverter.ZeroYieldDayOnMidnight = root["zero_day"] | false;
inverter.ClearEventlogOnMidnight = root["clear_eventlog"] | false;
inverter.YieldDayCorrection = root["yieldday_correction"] | false;

uint8_t arrayCount = 0;
for (JsonVariant channel : channelArray) {
inverter.channel[arrayCount].MaxChannelPower = channel["max_power"].as<uint16_t>();
inverter.channel[arrayCount].YieldTotalOffset = channel["yield_total_offset"].as<float>();
strncpy(inverter.channel[arrayCount].Name, channel["name"] | "", sizeof(inverter.channel[arrayCount].Name));
arrayCount++;
uint64_t old_serial = 0;

{
auto guard = Configuration.getWriteGuard();
auto& config = guard.getConfig();

INVERTER_CONFIG_T& inverter = config.Inverter[root["id"].as<uint8_t>()];

old_serial = inverter.Serial;
inverter.Serial = new_serial;
strncpy(inverter.Name, root["name"].as<String>().c_str(), INV_MAX_NAME_STRLEN);

inverter.Poll_Enable = root["poll_enable"] | true;
inverter.Poll_Enable_Night = root["poll_enable_night"] | true;
inverter.Command_Enable = root["command_enable"] | true;
inverter.Command_Enable_Night = root["command_enable_night"] | true;
inverter.ReachableThreshold = root["reachable_threshold"] | REACHABLE_THRESHOLD;
inverter.ZeroRuntimeDataIfUnrechable = root["zero_runtime"] | false;
inverter.ZeroYieldDayOnMidnight = root["zero_day"] | false;
inverter.ClearEventlogOnMidnight = root["clear_eventlog"] | false;
inverter.YieldDayCorrection = root["yieldday_correction"] | false;

uint8_t arrayCount = 0;
for (JsonVariant channel : channelArray) {
inverter.channel[arrayCount].MaxChannelPower = channel["max_power"].as<uint16_t>();
inverter.channel[arrayCount].YieldTotalOffset = channel["yield_total_offset"].as<float>();
strncpy(inverter.channel[arrayCount].Name, channel["name"] | "", sizeof(inverter.channel[arrayCount].Name));
arrayCount++;
}
}

WebApi.writeConfig(retMsg, WebApiError::InverterChanged, "Inverter changed!");

WebApi.sendJsonResponse(request, response, __FUNCTION__, __LINE__);

INVERTER_CONFIG_T const& inverter = Configuration.get().Inverter[root["id"].as<uint8_t>()];
std::shared_ptr<InverterAbstract> inv = Hoymiles.getInverterBySerial(old_serial);

if (inv != nullptr && new_serial != old_serial) {
Expand Down Expand Up @@ -300,7 +305,7 @@ void WebApiInverterClass::onInverterDelete(AsyncWebServerRequest* request)
}

uint8_t inverter_id = root["id"].as<uint8_t>();
INVERTER_CONFIG_T& inverter = Configuration.get().Inverter[inverter_id];
INVERTER_CONFIG_T const& inverter = Configuration.get().Inverter[inverter_id];

Hoymiles.removeInverterBySerial(inverter.Serial);

Expand Down Expand Up @@ -337,13 +342,18 @@ void WebApiInverterClass::onInverterOrder(AsyncWebServerRequest* request)
// The order array contains list or id in the right order
JsonArray orderArray = root["order"].as<JsonArray>();
uint8_t order = 0;
for (JsonVariant id : orderArray) {
uint8_t inverter_id = id.as<uint8_t>();
if (inverter_id < INV_MAX_COUNT) {
INVERTER_CONFIG_T& inverter = Configuration.get().Inverter[inverter_id];
inverter.Order = order;
{
auto guard = Configuration.getWriteGuard();
auto& config = guard.getConfig();

for (JsonVariant id : orderArray) {
uint8_t inverter_id = id.as<uint8_t>();
if (inverter_id < INV_MAX_COUNT) {
INVERTER_CONFIG_T& inverter = config.Inverter[inverter_id];
inverter.Order = order;
}
order++;
}
order++;
}

WebApi.writeConfig(retMsg, WebApiError::InverterOrdered, "Inverter order saved!");
Expand Down
Loading

0 comments on commit b1edb13

Please sign in to comment.