From 4acafea1b0a1bab58ea8297793c62ed8e641c571 Mon Sep 17 00:00:00 2001 From: Bartosz Wieczorek Date: Fri, 25 Jul 2025 11:08:58 +0200 Subject: [PATCH] add more checks --- rims_app/CMakeLists.txt.user | 74 ++++++++++++++++++----- rims_app/src/messenger.hpp | 11 ++-- rims_app/src/operation.cpp | 46 ++++++++------ rims_app/src/operation.hpp | 20 +++--- rims_app/src/temperature_measurements.cpp | 30 ++++++--- rims_app/src/temperature_measurements.hpp | 13 ++-- 6 files changed, 133 insertions(+), 61 deletions(-) diff --git a/rims_app/CMakeLists.txt.user b/rims_app/CMakeLists.txt.user index 3f4a9916d2c..13282089a2b 100644 --- a/rims_app/CMakeLists.txt.user +++ b/rims_app/CMakeLists.txt.user @@ -1,6 +1,6 @@ - + EnvironmentId @@ -109,15 +109,15 @@ 2 false - -DCMAKE_GENERATOR:STRING=Ninja + -DCMAKE_PREFIX_PATH:PATH=%{Qt:QT_INSTALL_PREFIX} -DQT_QMAKE_EXECUTABLE:FILEPATH=%{Qt:qmakeExecutable} --DCMAKE_BUILD_TYPE:STRING=Build +-DQT_MAINTENANCE_TOOL:FILEPATH=/opt/Qt/MaintenanceTool -DCMAKE_CXX_COMPILER:FILEPATH=%{Compiler:Executable:Cxx} --DCMAKE_COLOR_DIAGNOSTICS:BOOL=ON --DCMAKE_PREFIX_PATH:PATH=%{Qt:QT_INSTALL_PREFIX} -DCMAKE_PROJECT_INCLUDE_BEFORE:FILEPATH=%{BuildConfig:BuildDirectory:NativeFilePath}/.qtc/package-manager/auto-setup.cmake --DCMAKE_C_COMPILER:FILEPATH=%{Compiler:Executable:C} --DQT_MAINTENANCE_TOOL:FILEPATH=/opt/Qt/MaintenanceTool +-DCMAKE_COLOR_DIAGNOSTICS:BOOL=ON +-DCMAKE_BUILD_TYPE:STRING=Build +-DCMAKE_GENERATOR:STRING=Ninja +-DCMAKE_C_COMPILER:FILEPATH=%{Compiler:Executable:C} /home/bartoszek/zephyrproject/zephyr/rims_app /home/bartoszek/zephyrproject/zephyr/rims_app/build @@ -221,14 +221,36 @@ false -e cpu-cycles --call-graph dwarf,4096 -F 250 - - ProjectExplorer.CustomExecutableRunConfiguration - + zephyr_final + CMakeProjectManager.CMakeRunConfiguration. + zephyr_final false true + true true + /home/bartoszek/zephyrproject/zephyr/rims_app/build/zephyr - 1 + + true + true + 0 + true + + + 2 + + false + -e cpu-cycles --call-graph dwarf,4096 -F 250 + zephyr_pre0 + CMakeProjectManager.CMakeRunConfiguration. + zephyr_pre0 + false + true + true + true + /home/bartoszek/zephyrproject/zephyr/rims_app/build/zephyr + + 2 1 @@ -289,14 +311,36 @@ false -e cpu-cycles --call-graph dwarf,4096 -F 250 - - ProjectExplorer.CustomExecutableRunConfiguration - + zephyr_final + CMakeProjectManager.CMakeRunConfiguration. + zephyr_final false true + true true + /home/bartoszek/zephyrproject/zephyr/rims_app/build/zephyr - 1 + + true + true + 0 + true + + + 2 + + false + -e cpu-cycles --call-graph dwarf,4096 -F 250 + zephyr_pre0 + CMakeProjectManager.CMakeRunConfiguration. + zephyr_pre0 + false + true + true + true + /home/bartoszek/zephyrproject/zephyr/rims_app/build/zephyr + + 2 diff --git a/rims_app/src/messenger.hpp b/rims_app/src/messenger.hpp index 909bae99cf3..a91d90cb2cd 100644 --- a/rims_app/src/messenger.hpp +++ b/rims_app/src/messenger.hpp @@ -26,7 +26,7 @@ struct CommunicationCallback { /* * Sends bytes back. Bytes represents top level Egress message from each */ - virtual std::expected send(std::span submessage) const = 0; + NDIS virtual std::expected send(std::span submessage) const = 0; }; struct UartCallback : public CommunicationCallback { @@ -36,7 +36,7 @@ struct UartCallback : public CommunicationCallback { const pb_msgdesc_t *_fields; // fields of egress message WireFormatID _sender; - std::expected send(std::span subsystemMessage) const override; + NDIS std::expected send(std::span subsystemMessage) const override; }; struct IPCCallback : public CommunicationCallback { @@ -45,14 +45,14 @@ struct IPCCallback : public CommunicationCallback { function subsystemMessage)> _copy; std::reference_wrapper _sem; - std::expected send(std::span subsystemMessage) const override; + NDIS std::expected send(std::span subsystemMessage) const override; }; struct NoCallback : public CommunicationCallback { // CommunicationCallback interface public: - std::expected send(std::span submessage) const override { + NDIS std::expected send(std::span submessage) const override { return {}; } }; @@ -147,7 +147,7 @@ class MessageDispatcher { } }; - std::array _activeRequests; + std::array _activeRequests; zephyr::mutex::ZepryrSpinlock _lock; }; @@ -195,6 +195,5 @@ class MessengerThread : public ZephyrThread { std::array _events; public: - }; } // namespace rims diff --git a/rims_app/src/operation.cpp b/rims_app/src/operation.cpp index f453e966c45..9df05a6d694 100644 --- a/rims_app/src/operation.cpp +++ b/rims_app/src/operation.cpp @@ -1,12 +1,12 @@ #include "operation.hpp" #include "common.hpp" +#include "ctrl.hpp" #include "error.hpp" #include "inplace_vector.hpp" #include "log.hpp" #include "messenger.hpp" #include "pid.hpp" #include "zephyr.hpp" -#include "ctrl.hpp" #include #include @@ -73,26 +73,28 @@ void OperationOrchestrator::event_tick() { zephyr::semaphore::k_sem_take_now(_tickSem); ULOG_INFO("PID update thread"); for (auto &channel : _channels) { - channel.update(); + if (auto r = channel.update(); not r){ + ULOG_WARNING("Operation Orchestrator failed to 'make a tick' for channel %d with err %d/%d", channel.id(), r.error().endpoint, r.error().code); + } } } void OperationOrchestrator::event_operationMessageArrived() { static op_IngressMessage req; static op_EgressMessage resp; - constexpr auto service = "operation"; - + constexpr auto service = "operation"; + ULOG_INFO("%s consume ingress", service); operationIngressQueue.try_consume([&](const auto &_req) { req = _req; return true; }); - + ULOG_INFO("%s execute handler", service); for (const auto &handler : op_handlers) { if (handler.execute(this, req, resp)) break; } - + ULOG_INFO("%s produce egress", service); operationEgressQueue.try_produce(resp); } @@ -116,6 +118,7 @@ std::expected OperationOrchestrator::handle_initializeChannelRe const auto ch = req.ctrl_channels[i]; for (const auto &opchannel : _channels) { if (opchannel.usesControlChannel(ch)) { + ULOG_WARNING("opchannel %d alreadu uses channel %d", opchannel.id(), ch); return std::unexpected{op::channel_used_multiple_times{}}; } } @@ -131,13 +134,16 @@ std::expected OperationOrchestrator::handle_initializeChannelRe bool used = std::any_of(_channels.begin(), _channels.end(), [id](const OperationChannel &ch) { return ch.id() == id; }); if (!used) return id; } + ULOG_WARNING("There are no more valid channel IDs"); return std::unexpected{op::max_number_of_channels{}}; }; // create OperationChannel for channels and id - auto activatedChannels = TRY(OperationChannel::check_channels(std::span{channels})); - auto id = TRY(find_id()); - const auto &newChannel = _channels.emplace_back(activatedChannels, id); + auto activatedChannels = TRY(OperationChannel::check_channels(std::span{channels})); + ULOG_DEBUG("all ctrl channels seams to be active"); + auto id = TRY(find_id()); + const auto &newChannel = _channels.emplace_back(activatedChannels, id); + ULOG_INFO("created channel %d", newChannel.id()); resp.has_data = true; resp.data.channel = newChannel.id(); @@ -169,10 +175,11 @@ std::expected OperationOrchestrator::handle_modeRequest(const M return {}; } -TemperatureSlopeMode::TemperatureSlopeMode(PowerControl &pc, PIDController &pid, const op_TemperatureSlopeConfig &config) : _powerControl{pc}, _pid{pid}, _tempChannel{static_cast(config.temperature_channel_id)}, _slope{config.slope} { +TemperatureSlopeMode::TemperatureSlopeMode(PowerControl &pc, PIDController &pid, const op_TemperatureSlopeConfig &config) // + : _powerControl{pc}, _pid{pid}, _tempChannel{static_cast(config.temperature_channel_id)}, _slope{config.slope} { } -void TemperatureSlopeMode::update() { +std::expected TemperatureSlopeMode::update() { float currentTemperature = temp(_tempChannel).temperature_c; // Use simple hysteresis to reduce mode-switch jitter @@ -185,7 +192,7 @@ void TemperatureSlopeMode::update() { _heating = false; } const float outputPower = _pid.get().update(targetTemperature().temperature_c, currentTemperature); - _powerControl.get().setPower(outputPower); + return _powerControl.get().setPower(outputPower); }; auto useConstantSlope = [&]() { @@ -199,13 +206,13 @@ void TemperatureSlopeMode::update() { const float measuredSlope = (dT / dt) * 60.0f; // °C/min const float outputPower = _pid.get().update(_slope, measuredSlope); - _powerControl.get().setPower(outputPower); + return _powerControl.get().setPower(outputPower); }; if (closeToSetpoint) { - useConstantTemperatureAlg(); + return useConstantTemperatureAlg(); } else { - useConstantSlope(); + return useConstantSlope(); } _lastTemperature = currentTemperature; @@ -227,10 +234,11 @@ ConstantTemperatureMode::ConstantTemperatureMode(PowerControl &pc, PIDController } } -void ConstantTemperatureMode::update() { +std::expected ConstantTemperatureMode::update() { auto currentTemperature = temp(_tempChannel).temperature_c; float outputPower = _pid.get().update(targetTemperature().temperature_c, currentTemperature); - _powerControl.get().setPower(outputPower); + CHECK(_powerControl.get().setPower(outputPower)); + return {}; } std::expected OperationOrchestrator::handle_targetTemperatureRequest(const TargetTemperatureRequest &req, TargetTemperatureResponse &resp) { @@ -349,7 +357,7 @@ PowerControl::PowerControl(std::span channels) : _control_channels{chan ULOG_DEBUG("PowerControl start"); } -void PowerControl::setPower(float percent) { +std::expected PowerControl::setPower(float percent) { ULOG_INFO("Set power %f", (double)percent); for (const auto ch : _control_channels) { @@ -360,6 +368,8 @@ void PowerControl::setPower(float percent) { MessengerThread::ipc_request(plr); } + + return {}; } } // namespace rims diff --git a/rims_app/src/operation.hpp b/rims_app/src/operation.hpp index 2381aabb0f5..fcb768677e5 100644 --- a/rims_app/src/operation.hpp +++ b/rims_app/src/operation.hpp @@ -87,7 +87,7 @@ class PowerControl { static std::expected check_channels(std::span channels); PowerControl(std::span channels); - void setPower(float percent); + NDIS std::expected setPower(float percent); bool usesChannel(uint8_t ch) const { return std::find(_control_channels.begin(), _control_channels.end(), ch) != _control_channels.end(); @@ -108,7 +108,8 @@ class DisabledMode : public ModeStrategy { public: DisabledMode() { } - void update() { + NDIS std::expected update() { + return {}; } float targetTemperature() const = delete; void setTargetTemperature(float temp) = delete; @@ -125,7 +126,7 @@ class ConstantTemperatureMode : public ModeStrategy { ConstantTemperatureMode(ConstantTemperatureMode &&) = default; ConstantTemperatureMode &operator=(ConstantTemperatureMode &&) = default; - void update(); + NDIS std::expected update(); private: /// reference wrapper allows this class to be movable @@ -145,7 +146,7 @@ class TemperatureSlopeMode : public ModeStrategy { TemperatureSlopeMode(TemperatureSlopeMode &&) = default; TemperatureSlopeMode &operator=(TemperatureSlopeMode &&) = default; - void update(); + NDIS std::expected update(); private: /// reference wrapper allows this class to be movable @@ -209,8 +210,8 @@ class OperationChannel { return _ownId; } - void update() { - std::visit([](auto &mode) { mode.update(); }, _mode); + NDIS std::expected update() { + return std::visit([](auto &mode) { return mode.update(); }, _mode); } NDIS std::expected handle_targetTemperatureRequest(const TargetTemperatureRequest &request, TargetTemperatureResponse &resp); @@ -247,7 +248,10 @@ class OperationOrchestrator { NDIS std::expected channelValid(uint8_t ch) const { auto found = std::find_if(_channels.begin(), _channels.end(), [&](const OperationChannel &opch) { return opch.id() == ch; }); - if (found != _channels.end()) return std::unexpected{op::channel_not_found{}}; + if (found == _channels.end()) { + ULOG_WARNING("channel %d not found", ch); + return std::unexpected{op::channel_not_found{}}; + } return {}; } @@ -260,6 +264,7 @@ class OperationOrchestrator { auto found = std::find_if(_channels.begin(), _channels.end(), [&](const OperationChannel &opch) { return opch.id() == ch; }); if (found == _channels.end()) { + ULOG_WARNING("channel %d not found", ch); return std::unexpected{op::channel_not_found{}}; } @@ -270,6 +275,7 @@ class OperationOrchestrator { auto found = std::find_if(_channels.begin(), _channels.end(), [&](const OperationChannel &opch) { return opch.id() == ch; }); if (found == _channels.end()) { + ULOG_WARNING("channel %d not found", ch); return std::unexpected{op::channel_not_found{}}; } diff --git a/rims_app/src/temperature_measurements.cpp b/rims_app/src/temperature_measurements.cpp index 2a6c82e1b82..a5a3ad9e331 100644 --- a/rims_app/src/temperature_measurements.cpp +++ b/rims_app/src/temperature_measurements.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -92,7 +93,9 @@ TemperatureSampler::TemperatureSampler(uint8_t channel) : _temperature{0.05}, _c k_work_init(&_sampleWork, TemperatureSampler::sample_work_handler); select_channel(_channel); - calibration(); + if (not calibration()) { + ULOG_WARNING("Temperature sampler channel %d calibration failed", _channel); + } _broadcastTimer.start(); _samplerTimer.start(); @@ -227,6 +230,8 @@ float TemperatureSampler::adc2CelciusWeight() const { const auto adcMax = this->_referenceAdcMax.filtered(); const auto adcMin = this->_referenceAdcMin.filtered(); + assert(adcMin != adcMax); + return (tempMax - tempMin) / static_cast(adcMin - adcMax); } @@ -303,15 +308,25 @@ std::expected TemperatureSampler::adc() { constexpr auto ADC_RESOLUTION = 12; const adc_dt_spec adc_spec = ADC_DT_SPEC_GET_BY_IDX(DT_PATH(zephyr_user), 0); const auto nsamples = oversampling(); - const adc_sequence_options options = { - .interval_us = 100, - .callback = nullptr, - .user_data = nullptr, - .extra_samplings = static_cast(nsamples - 1), + + const adc_sequence_options options = { + .interval_us = 100, + .callback = nullptr, + .user_data = nullptr, + .extra_samplings = static_cast(nsamples - 1), }; samples.resize(nsamples, 0); - adc_sequence sequence = {.options = &options, .channels = BIT(15), .buffer = samples.data(), .buffer_size = samples.size() * sizeof(adc_sample), .resolution = ADC_RESOLUTION, .oversampling = 0, .calibrate = false}; + const adc_sequence sequence = {// + .options = &options, + .channels = BIT(15), + .buffer = samples.data(), + .buffer_size = samples.size() * sizeof(adc_sample), + .resolution = ADC_RESOLUTION, + .oversampling = 0, + .calibrate = false + }; + /* * @retval -EINVAL If a parameter with an invalid value has been provided. * @retval -ENOMEM If the provided buffer is to small to hold the results @@ -324,7 +339,6 @@ std::expected TemperatureSampler::adc() { * in the buffer, but at least some of them were taken with * an extra delay compared to what was scheduled. */ - auto ret = adc_read_dt(&adc_spec, &sequence); if (ret != 0) { if (not _faultTP) { diff --git a/rims_app/src/temperature_measurements.hpp b/rims_app/src/temperature_measurements.hpp index f49100f676b..8f464b32af7 100644 --- a/rims_app/src/temperature_measurements.hpp +++ b/rims_app/src/temperature_measurements.hpp @@ -111,7 +111,7 @@ class TemperatureSampler { TemperatureSampler &operator=(TemperatureSampler &&) = delete; void tick() ; - std::expected calibration() ; + NDIS std::expected calibration() ; bool pending() const { return _samplePending; @@ -134,8 +134,8 @@ class TemperatureSampler { float adc2CelciusWeight() const; Temperature_c adcToTemperature(uint32_t adc_value) const; - std::expected adcWithCalibration(); - std::expected adc(); + NDIS std::expected adcWithCalibration(); + NDIS std::expected adc(); void do_sample() ; struct k_work _sampleWork{}; @@ -191,14 +191,13 @@ class TemperatureSamplerOrchestrator { TemperatureSamplerOrchestrator(); void loop(); - [[nodiscard]] std::expected handle_getTemperatureRequest(const GetTemperatureRequest &req, GetTemperatureResponse &resp) const; - [[nodiscard]] std::expected handle_activeChannelsRequest(const GetActiveChannelsRequest &req, GetActiveChannelsResponse &resp) const; - [[nodiscard]] std::expected handle_samplerConfigRequest(const SamplerConfigRequest &req, SamplerConfigResponse &resp); + NDIS std::expected handle_getTemperatureRequest(const GetTemperatureRequest &req, GetTemperatureResponse &resp) const; + NDIS std::expected handle_activeChannelsRequest(const GetActiveChannelsRequest &req, GetActiveChannelsResponse &resp) const; + NDIS std::expected handle_samplerConfigRequest(const SamplerConfigRequest &req, SamplerConfigResponse &resp); private: // events handled in main loop void event_messageArrived(); - void action_samplersTick(); std::array _temperatureSamplerChannels;