Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions release-notes/smartfuel-config-hoist.build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- **SmartFuel no longer rebuilds its configuration and reset signature on every wake (`tasks/events/telemetry_bg/smart.lua`)**:
- `Smart.wakeup` computes once a second in every Lua state that runs the event runtime (the dashboard and service widgets, the tool). Each of those wakes built a new configuration table through three fresh closures and a parsed API version, although the configuration only changes when a setting does, and joined seven `tostring` results into a string to decide whether to restart the estimate, although those values only change with the settings, the battery configuration, the active battery profile or the pack's cell count. The helpers are now file-level functions, the configuration is rebuilt only when one of its input values differs from the last wake (compared by value, so a page writing into the session's tables in place is still seen), and the restart test compares the seven values field by field. Measured on desktop Lua 5.3, a computing wake with unchanged inputs allocates nothing instead of 945 bytes, and costs 510 instructions instead of 697 (firmware source) or 749 instead of 927 (local voltage source). The configuration and every restart decision are unchanged, with one exception: the restart values are now compared as numbers, so a value that changes only between an integer and a float of the same size (a cell count reported as `6.0` where it was `6`, a capacity of `2000.0` where it was `2000`) no longer restarts the estimate. The string comparison restarted it, because Lua 5.3 prints the two differently (#768).
161 changes: 108 additions & 53 deletions src/rfsuite/tasks/events/telemetry_bg/smart.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ local lastWake = 0
local wakeInterval = 1.0

local state = {
batterySignature = nil,
sourceMode = nil,
batterySignature = {},
signatureValid = false,
stabilizeUntil = 0,
voltageSamples = {},
voltageStable = false,
Expand Down Expand Up @@ -225,60 +225,105 @@ local function scaleField(raw, fallback, minValue, maxValue, scale)
return value
end

local function getSmartConfig(session)
local cfg = session and session.smartfuel_config or nil
local batteryPrefs = session and session.modelPreferences and session.modelPreferences.battery or nil

local function readLegacyLocalSource()
local source = batteryPrefs and tonumber(batteryPrefs.smartfuel_source) or nil
if source == nil then
source = batteryPrefs and tonumber(batteryPrefs.calc_local) or nil
end
return source
end
local FIRMWARE_SOURCE_MIN_API = { 12, 0, 9 }

local function getFirmwareSource()
local apiVersion = ApiVersion and ApiVersion.parse and ApiVersion.parse(session and session.apiVersion)
if not (ApiVersion and ApiVersion.isAtLeast and ApiVersion.isAtLeast(apiVersion, { 12, 0, 9 })) then
return nil
end
local mode = cfg and tonumber(cfg.smartfuel_mode) or nil
if mode == nil then
local batteryConfig = session and session.battery_config
mode = batteryConfig and tonumber(batteryConfig.smartfuelRemoteSource) or nil
end
return mode
local function readLegacyLocalSource(batteryPrefs)
local source = batteryPrefs and tonumber(batteryPrefs.smartfuel_source) or nil
if source == nil then
source = batteryPrefs and tonumber(batteryPrefs.calc_local) or nil
end
return source
end

local function pick(key)
if cfg and cfg[key] ~= nil then return cfg[key] end
if batteryPrefs and batteryPrefs[key] ~= nil then return batteryPrefs[key] end
local function getFirmwareSource(session, cfg)
local apiVersion = ApiVersion and ApiVersion.parse and ApiVersion.parse(session and session.apiVersion)
if not (ApiVersion and ApiVersion.isAtLeast and ApiVersion.isAtLeast(apiVersion, FIRMWARE_SOURCE_MIN_API)) then
return nil
end
local mode = cfg and tonumber(cfg.smartfuel_mode) or nil
if mode == nil then
local batteryConfig = session and session.battery_config
mode = batteryConfig and tonumber(batteryConfig.smartfuelRemoteSource) or nil
end
return mode
end

local function pick(cfg, batteryPrefs, key)
if cfg and cfg[key] ~= nil then return cfg[key] end
if batteryPrefs and batteryPrefs[key] ~= nil then return batteryPrefs[key] end
return nil
end

-- The configuration is a function of the thirteen values read at the top of getSmartConfig and of
-- nothing else, and they change only when a setting or the flight controller's configuration does
-- -- while the configuration is read on every wake that computes. So the values it was last built
-- from are kept here, and the table is rebuilt only when one of them differs. They are compared by
-- value, not by the identity of the tables they come from: the SmartFuel and battery pages write
-- into the session's tables in place.
local configInputs = {
apiVersion = false, mode = false, remoteSource = false, localSource = false, calcLocal = false,
publish = false, voltageDropRate = false, chargeDropRate = false, sagGain = false,
voltageFallLimit = false, fuelDropRate = false, stabilizeDelay = false, stableWindow = false,
}
local smartConfig = nil

local source = readLegacyLocalSource()
local function getSmartConfig(session)
local cfg = session and session.smartfuel_config or nil
local batteryPrefs = session and session.modelPreferences and session.modelPreferences.battery or nil
local batteryConfig = session and session.battery_config

local apiVersion = session and session.apiVersion
local mode = cfg and cfg.smartfuel_mode
local remoteSource = batteryConfig and batteryConfig.smartfuelRemoteSource
local localSource = batteryPrefs and batteryPrefs.smartfuel_source
local calcLocal = batteryPrefs and batteryPrefs.calc_local
local publish = batteryPrefs and batteryPrefs.smartfuel_publish
local voltageDropRate = pick(cfg, batteryPrefs, "voltage_drop_rate")
local chargeDropRate = pick(cfg, batteryPrefs, "charge_drop_rate")
local sagGain = pick(cfg, batteryPrefs, "sag_gain")
local voltageFallLimit = pick(cfg, batteryPrefs, "voltage_fall_limit")
local fuelDropRate = pick(cfg, batteryPrefs, "fuel_drop_rate")
local stabilizeDelay = pick(cfg, batteryPrefs, "stabilize_delay")
local stableWindow = pick(cfg, batteryPrefs, "stable_window")

local last = configInputs
if smartConfig
and last.apiVersion == apiVersion and last.mode == mode and last.remoteSource == remoteSource
and last.localSource == localSource and last.calcLocal == calcLocal and last.publish == publish
and last.voltageDropRate == voltageDropRate and last.chargeDropRate == chargeDropRate
and last.sagGain == sagGain and last.voltageFallLimit == voltageFallLimit
and last.fuelDropRate == fuelDropRate and last.stabilizeDelay == stabilizeDelay
and last.stableWindow == stableWindow then
return smartConfig

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cached table is now returned by reference. Its only caller is Smart.wakeup (:620), which reads source, firmwareSource, stabilizeDelaySeconds and stableWindowVolts and writes none of them, so the cache cannot be polluted today. If a later caller ever writes into cfg, this is the line where that becomes a stale-configuration bug.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. 35f641b puts that rule into the comment above the cache: the kept table is handed out itself, not a copy, so a caller must not write into it.

end
last.apiVersion, last.mode, last.remoteSource = apiVersion, mode, remoteSource
last.localSource, last.calcLocal, last.publish = localSource, calcLocal, publish
last.voltageDropRate, last.chargeDropRate, last.sagGain = voltageDropRate, chargeDropRate, sagGain
last.voltageFallLimit, last.fuelDropRate = voltageFallLimit, fuelDropRate
last.stabilizeDelay, last.stableWindow = stabilizeDelay, stableWindow

local source = readLegacyLocalSource(batteryPrefs)
if source == nil then source = 0 end

local voltageDropRate = tonumber(pick("voltage_drop_rate"))
local chargeDropRate = tonumber(pick("charge_drop_rate"))
local sagGain = tonumber(pick("sag_gain")) or 40
voltageDropRate = tonumber(voltageDropRate)
chargeDropRate = tonumber(chargeDropRate)

local voltageFallPerSecond = nil
if voltageDropRate ~= nil then
voltageFallPerSecond = voltageDropRate / 1000
else
voltageFallPerSecond = scaleField(pick("voltage_fall_limit"), 0.05, 0, 1, 100)
voltageFallPerSecond = scaleField(voltageFallLimit, 0.05, 0, 1, 100)
end

local fuelDropPerSecond = nil
if chargeDropRate ~= nil then
fuelDropPerSecond = chargeDropRate / 100
else
fuelDropPerSecond = scaleField(pick("fuel_drop_rate"), 1.0, 0, 50, 10)
fuelDropPerSecond = scaleField(fuelDropRate, 1.0, 0, 50, 10)
end

return {
firmwareSource = getFirmwareSource(),
smartConfig = {
firmwareSource = getFirmwareSource(session, cfg),
source = source,
-- Whether the two sensors are published at all. Read from the model's own preferences and
-- deliberately NOT through `pick`: `pick` lets the flight controller's SmartFuel
Expand All @@ -291,12 +336,13 @@ local function getSmartConfig(session)
-- on SmFt survives the update. Nothing inside the suite depends on it either way -- those
-- consumers are served by the hand-over below.
publishSensors = (batteryPrefs and batteryPrefs.smartfuel_publish) ~= false,
stabilizeDelaySeconds = scaleField(pick("stabilize_delay"), 1.5, 0, 10, 1000),
stableWindowVolts = scaleField(pick("stable_window"), 0.15, 0, 1, 100),
stabilizeDelaySeconds = scaleField(stabilizeDelay, 1.5, 0, 10, 1000),
stableWindowVolts = scaleField(stableWindow, 0.15, 0, 1, 100),
voltageFallPerSecond = voltageFallPerSecond,
fuelDropPerSecond = fuelDropPerSecond,
sagGain = sagGain
sagGain = tonumber(sagGain) or 40
}
return smartConfig
end

-- Firmware with per-profile battery cells (tasks/msp/api/battery_config.lua, `hasProfileCells`)
Expand Down Expand Up @@ -603,19 +649,28 @@ function Smart.wakeup()
return
end

local signature = table.concat({
tostring(firmwareSource or "nil"),
tostring(sourceMode),
tostring(cellCount),
tostring(packCapacity),
tostring(reserve),
tostring((tonumber(batteryConfig.vbatmincellvoltage) or 0)),
tostring((tonumber(batteryConfig.vbatfullcellvoltage) or 0))
}, ":")

if state.batterySignature ~= signature or state.sourceMode ~= sourceMode then
state.batterySignature = signature
state.sourceMode = sourceMode
-- The estimate restarts when any of these differs from the values it was started with. Compared
-- field by field: joining them into a string to compare cost a table and seven strings on every
-- wake that got this far.
local minCellVoltage = tonumber(batteryConfig.vbatmincellvoltage) or 0
local fullCellVoltage = tonumber(batteryConfig.vbatfullcellvoltage) or 0
local signature = state.batterySignature
if not state.signatureValid
or signature.firmwareSource ~= firmwareSource
or signature.sourceMode ~= sourceMode
or signature.cellCount ~= cellCount
or signature.packCapacity ~= packCapacity
or signature.reserve ~= reserve
or signature.minCellVoltage ~= minCellVoltage
or signature.fullCellVoltage ~= fullCellVoltage then
signature.firmwareSource = firmwareSource
signature.sourceMode = sourceMode
signature.cellCount = cellCount
signature.packCapacity = packCapacity
signature.reserve = reserve
signature.minCellVoltage = minCellVoltage
signature.fullCellVoltage = fullCellVoltage
state.signatureValid = true
state.stabilizeUntil = now + cfg.stabilizeDelaySeconds
resetComputedState()
logSmart("smart reset source=" .. tostring(sourceMode) .. " cap=" .. tostring(packCapacity), "info")
Expand Down Expand Up @@ -687,8 +742,8 @@ end

function Smart.reset()
clearHandOver()
state.batterySignature = nil
state.sourceMode = nil
state.signatureValid = false
smartConfig = nil
state.stabilizeUntil = 0
state.lastFuelValue = nil
state.lastFuelPush = 0
Expand Down
Loading