Skip to content

webserver: stop the file editor buffering whole files into internal RAM - #1504

Open
kezarjg wants to merge 1 commit into
openvehicles:masterfrom
kezarjg:feature/webserver-extram-sender
Open

webserver: stop the file editor buffering whole files into internal RAM#1504
kezarjg wants to merge 1 commit into
openvehicles:masterfrom
kezarjg:feature/webserver-extram-sender

Conversation

@kezarjg

@kezarjg kezarjg commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

The web file editor at /api/file serves any readable path under /store or /sd, so the body is arbitrarily large. On a GET, HandleFile passed the whole body to PageContext::print(), which appends it to the connection send buffer, and that buffer cannot drain while the handler is still running. So the file exists twice for the duration: once in the extram::string it was loaded into, which is SPIRAM, and once as an mbuf copy in internal 8-bit RAM, which is the scarce pool.

This streams the body straight out of the buffer it was loaded into, so the second copy never happens.

Evidence

Browsing stored logs in the editor failed reproducibly once a session had served roughly 660 KB across about two dozen files. The failing allocation was usually somewhere unrelated, because what ran out was internal RAM in general, so the visible symptom moved between runs. After the change the same walk completed 72 files and about 20 MB cleanly.

Measured on an OVMS v3 module reading SD-card logs over the local network. Those runs were on my own tree, which carries this change plus unrelated work: the code path is identical, but the module hours are not attributable to this branch alone.

The change

HandleFile's GET path hands ownership of the loaded body to a chunked sender and returns. The sender frees the body and itself after the last chunk and emits the terminating zero-length chunk, which is why this path deliberately does not fall through to c.done(). An empty body, meaning a directory path, is handled: the sender terminates on its first event.

HttpStringSender is generalised to HttpStringSenderT<StringType>:

typedef HttpStringSenderT<std::string>      HttpStringSender;
typedef HttpStringSenderT<extram::string>   HttpExtRamStringSender;

The old name survives as a typedef, so this is source compatible, and nothing else in the tree referenced the class, so HandleFile is the only behavioural change. The implementation moves into the header because it is now a template, which is a mechanical move with the ESP_EARLY_LOGV traces preserved verbatim (the log tag is the literal "webserver"). The one functional edit inside the sender is computing the chunk length with a comparison rather than the MIN macro, which is not visible in the header.

Testing

Builds clean in CI with no vehicle modules enabled, so the result is attributable to this change alone. That covers compilation; the figures above are the behavioural evidence.

Worth knowing

The streaming path is roughly 1.6x slower than serving the same file statically. Irrelevant when editing a script, but a client pulling a multi-megabyte file through /api/file needs a timeout above about 150 seconds, and a default-timeout client may now give up where it previously succeeded.

I could not find an existing issue for this. It surfaced while chasing an unrelated memory problem rather than from a user report.

Three decisions I would happily revisit

Only HandleFile is converted. The other c.print() sites on an extram::string are PluginHandler, which holds a reference to registry-owned content and so cannot pass ownership without a copy, and PluginCallback, which emits a fragment into a composed page where a stream-and-return sender would truncate everything after it. Plugin content is also bounded in practice, unlike an arbitrary path under /sd. Happy to add a non-owning variant if you want those covered.

Template versus a second class. Duplicating forty lines to change one type seemed the worse trade, but this does touch code that already works. Easy to switch to a standalone extram sender alongside the concrete class if you prefer.

Streaming is unconditional. Small files pay the slowdown for a problem they do not have. A size threshold avoids that at the cost of two paths to keep correct, and I erred toward one.

HandleFile serves any readable path under /store or /sd to the web file editor, so the body it returns is arbitrarily large - a log file or a stored report can be several megabytes. The GET path passed the whole body to PageContext::print(), which appends it to the connection send buffer. That buffer cannot drain while the handler is still running, so for the duration of the handler the file exists twice: once in the extram::string it was loaded into, and once again in the mbuf copy in internal RAM. The second copy is the problem, because internal 8-bit RAM is the scarce pool.

In practice this is a crash, not just a high-water mark. Walking a directory of stored files in the editor died reproducibly once the session had served roughly 660 KB across about two dozen files; the allocation that failed was usually somewhere unrelated, because what ran out was internal RAM in general rather than anything belonging to the webserver. After switching this handler to streaming, the same walk completed 72 files and about 20 MB with no failures. Both figures were measured on an OVMS v3 module reading logs from its SD card, driving the editor from a browser on the local network; the pre-fix failure point moved around a little between runs, as you would expect from a general internal-RAM exhaustion rather than a single oversized allocation.

The fix hands ownership of the already-loaded body to a chunked sender and returns without calling c.done(), so the body is transmitted in XFER_CHUNK_SIZE pieces straight out of the buffer it was loaded into and is never copied into internal RAM at all. The sender frees the body and itself once the last chunk has gone out and emits the terminating zero-length chunk, which is why this path deliberately does not fall through to c.done(). An empty body - a directory path, which loads nothing - is handled correctly: the sender emits the terminating chunk on its first event and retires.

To make that possible, HttpStringSender is generalised into HttpStringSenderT<StringType> with two instantiations: HttpStringSender for a std::string, exactly as before, and HttpExtRamStringSender for an extram::string. The existing name is preserved as a typedef, so this is source compatible for any caller; the implementation moves from the .cpp into the header because it is now a template, and the existing ESP_EARLY_LOGV traces are preserved verbatim. Nothing else in the tree referenced the class, so the only behavioural change is the one described above.

One operational note for anyone timing this: the streaming path is roughly 1.6x slower than serving the same file statically, so a client fetching a multi-megabyte file through the editor needs a timeout above about 150 seconds.
kezarjg added a commit to kezarjg/Open-Vehicle-Monitoring-System-3 that referenced this pull request Aug 22, 2026
kezarjg added a commit to kezarjg/Open-Vehicle-Monitoring-System-3 that referenced this pull request Aug 22, 2026
Adds support for Toyota's e-TNGA electric platform via the OBD-II port, as a shared platform module plus two thin vehicle wrappers: OvmsVehicleSubaruSolterra ("SUBSOL") and OvmsVehicleToyotaBz4x ("TOYBZ4X"), both deriving from OvmsVehicleToyotaETNGA. The two cars are the same vehicle electrically, so all of the polling, the charging state machine and the BMS handling live in the base and neither wrapper carries any behaviour of its own beyond registering itself. The base is not directly selectable; it compiles when either vehicle is enabled.

I could not find an existing base-and-derived vehicle family in the tree to copy, so if you would rather see this as one component with a model switch, or structured some other way, say so and I will rework it.

Supported: VIN, SOC as displayed and as the BMS reports it, speed, odometer, gear, drive mode, per-cell BMS voltages and temperatures, battery capacity and state of health, AC and DC charging including type, power, station and vehicle limits and charge port state, TPMS via the gateway relay, 12V auxiliary battery voltage, current, temperature and capacity from the EV ECU, cabin and ambient temperature, HVAC power and per-trip cabin energy, and throttle, brake and park brake position. Charge control, climate control and lock/unlock are not implemented.

Three things worth knowing before reading the code:

The poll list is split into two series. The platform runs seven poll states (sleep, awake, driving, and four charge states) but VEHICLE_POLL_NSTATES is 4, so obdii_polls_base covers states 0-2 and obdii_polls_charge is registered at an offset of CHARGE_HANDSHAKE to cover states 3-6. A PID needed on both sides appears in both arrays with only its own side's cadences. This follows the secondary-series pattern the Hyundai Ioniq 5 uses. The fourth column of the base array must stay zero because it aliases the first charge state.

The pack arrangement is derived at runtime rather than declared per vehicle. e-TNGA packs vary by model year and drivetrain, not by badge: 96 cells on 2022-24 cars, 78 or 104 on the 2025/26 refresh depending on drivetrain. The base declares a bootstrap 96-cell arrangement and then re-derives the cell and sensor counts from the length of the 0x182E and 0x1814 replies. The derivation is deliberately grow-only, because a short reply is indistinguishable from a truncated one.

Only the Solterra has been confirmed on hardware, and only the 96-cell pack. The module is developed against one car. The 78 and 104 cell packs are reasoned from published specifications with no hardware behind them, and no e-TNGA behaviour at all has been exercised on a bZ4X. The vehicle documentation carries a validation status table recording, per behaviour, whether it was observed on a car and when, or inferred from a specification or an analogous DID.

Flash cost is about 76 KB across the three components, with 12 bytes of static DRAM and no IRAM at all.

This depends on PR openvehicles#1504 (webserver: stop the file editor buffering whole files into internal RAM). The charge-report browser serves stored session CSVs, which run to several megabytes, and it uses the HttpExtRamStringSender that openvehicles#1504 introduces. Without that merged first this will not compile.

Some data-collection metrics carried on my own branch are deliberately not here: the raw per-module capacity arrays, and two lifetime counters from 0x1D70 whose units are unresolved. They exist to answer open reverse-engineering questions and have no value to a user, so they are not part of this submission. The 0x1D3E poll behind them stays, because v.b.cac is derived from that reply.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant