Skip to content

Free RAM on every board - #99

Merged
tridge merged 7 commits into
ArduPilot:masterfrom
julianoes:julianoes/ram-savings
Oct 4, 2026
Merged

tridge merged 7 commits into
ArduPilot:masterfrom
julianoes:julianoes/ram-savings

Conversation

@julianoes

@julianoes julianoes commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Five small changes that free RAM on every board, without changing behaviour on the air or the parameters. RAM is the binding constraint on this firmware: before these, the 8KB boards had 9 of their 256 byte pdata page left, and 3dr1060/hb1060/ism01a had 13 bytes of XRAM.

board pdata xdata end free RAM flash freed
3dr1060 / hb1060 / ism01a 191 → 174 0x0ff2 → 0x0fc1 13 → 62 B ~2.7 KB
hm_trp / rf50 / rfd900 186 → 169 0x0fdc → 0x0fab 35 → 84 B ~2.7 KB
rfd900a 186 → 169 0x0fe9 → 0x0fb8 22 → 71 B ~2.7 KB
mro900, rfd900p/pe, rfd900u/ue (8KB) 247 → 169 plenty spare pdata 9 → 87 B ~2.3 KB

Measured from obj/<board>/radio~<board>/radio~<board>.mem with sdcc 4.2.0, before and after. All 12 boards build and tests/test_build.sh passes.

The changes

  • at: the long AT command buffer is only for AES builds. AT_CMD_MAXLEN is 69 instead of 16 on CPU_SI1030 boards, for the 64 character key of AT&E=, which is inside #ifdef INCLUDE_AES. Asking for that instead of the CPU shrinks at_cmd and the remote command buffer together: 53 bytes of pdata and 53 of xdata on the 8KB boards, identical generated code. AES builds keep the long buffer.
  • printfl: convert longs in place. vprintfl() always calls __ultoa()/__ltoa(), whose 32 byte buffer is statically allocated in xdata in --model-large: 49 bytes, while printfl already has a 12 byte buffer for the same purpose. Uppercase hex and the minus sign for radix 10 match SDCC's behaviour, and -2147483648 still fits. 49 bytes of xdata and 257 of flash on every board, which is what the three tightest boards needed.
  • serial: the encrypt ring pointers are only for AES builds. They are declared for CPU_SI1030 but every use is inside #ifdef INCLUDE_AES. 8 bytes of pdata on the 8KB boards.
  • serial: no zeroed image of the serial buffers. = {0} on two ring buffers that are written before they are read only moves them to XISEG and puts an all zero image in flash for the startup code to copy. Same RAM either way, 2495 bytes of flash on the 4KB boards and 2048 on the others.
  • tdm: the remote AT command buffer leaves the pdata page. It only goes through memcpy() and strlen(), so it does not need the cheaper addressing. 17 bytes of pdata on the 8KB boards, where that page is the limit; no change on the 4KB boards, where pdata and xdata share the same memory.

The AT command limit is user visible on the five CPU_SI1030 boards: it drops from 69 to 16 characters, since INCLUDE_AES is never defined to the compiler today. Nothing overflows (at_input() bounds the write, handle_at_command() rejects an over-long remote command), but a local command longer than the limit drops the radio out of AT mode back into passthrough. The longest commands in the tree are AT&UPDATE and AT&T=RSSI at 9 characters and ATS15=4294967295 at exactly 16.

Nothing here removes AES code; the AES paths keep their buffers and are only guarded by #ifdef INCLUDE_AES as they already were elsewhere. Note that INCLUDE_AES is currently never defined to the compiler (include/rules.mk only adds the sources to the include path), so those paths are not built today and CI does not exercise them.

Noted while looking, not changed here

  • Nothing checks the 256 byte pdata page at link time: tools/check_code.py only checks XISEG against XRAM_SIZE. The pdata_canary in at.c catches an overflow at runtime, but only because at.rel happens to link first. An explicit check would be worth adding.
  • average_duty_cycle in tdm.c is the only float in the firmware and drags in the float library, about 1 KB of flash for 2 bytes of RAM. An integer filter would replace it, but it changes the duty cycle arithmetic, so it is not in this PR.
  • The largest remaining lever for the 4KB boards would be replacing last_received (252 bytes, packet.c) with a CRC for the duplicate check. That trades a 1 in 65536 chance of dropping a genuine resent packet, so it wants its own discussion.

AT_CMD_MAXLEN is 69 rather than 16 on CPU_SI1030 boards. The only command
that long is AT&E= with a 64 character AES key, which is inside
#ifdef INCLUDE_AES, so ask for that instead of the CPU. It sizes both
at_cmd and the remote command buffer in tdm.c, so on mro900, rfd900p/pe
and rfd900u/ue this frees 53 bytes of the 256 byte pdata page and 53 bytes
of xdata, with the generated code unchanged.

AES builds keep the long buffer.
vprintfl() always calls __ultoa()/__ltoa(), whatever the format string.
SDCC's versions carry their own 32 byte buffer, which in --model-large is
statically allocated in xdata, 49 bytes in total, and printfl already has a
12 byte buffer of its own for exactly this.

Emit the digits backwards into that buffer instead. Uppercase hex and the
minus sign for radix 10 match what SDCC's versions do, and the longest
result, -2147483648, still fits. Frees 49 bytes of xdata and 257 bytes of
flash on every board.
encrypt_buff_start/end and encrypt_insert/remove are declared for
CPU_SI1030, but every use of them is inside #ifdef INCLUDE_AES, apart from
a reset in serial_init() that does nothing without them. Ask for the same
thing the users do. Frees 8 bytes of the 256 byte pdata page on mro900,
rfd900p/pe and rfd900u/ue.
rx_buf and tx_buf are ring buffers, written before they are read, and
their insert and remove indices are set in serial_init(). Initialising
them to {0} only moves them from XSEG to XISEG and puts a matching all
zero image of 2495 bytes (2048 on CPU_SI1030 boards) in flash, which the
startup code then copies. RAM use is the same either way.

Frees 2495 bytes of flash on hm_trp and the other 4KB boards, 2048 on the
8KB ones.
remote_at_cmd only ever goes through memcpy() and strlen(), so it does not
need to be in the 256 byte pdata page that the CPU can address more
cheaply. On mro900, rfd900p/pe and rfd900u/ue that page is the binding
limit, while xdata has kilobytes to spare; on the 4KB boards the two share
the same memory, so nothing changes there. The generated code is
identical.
@AP-Review

AP-Review commented Sep 20, 2026 •

Copy link
Copy Markdown

Deprecated — see below for the updated review.

Previous review (2026-09-20)

Automated review note — AI-generated (Claude), validated against the live diff (Claude + Codex cross-checked). Please sanity-check before acting.

Full report: https://uav.tridgell.net/DevCallReviews/2026_09_20/devcall_pr_reviews.html#prSiK-99

Verdict: COMMENT at head f97dea41b3. CI green. The code changes are sound and every number in your table reproduces — all twelve boards were built here from master and from your head with sdcc 4.5.0, tests/test_build.sh rc=0 at both. One thing is worth fixing before this lands.

The savings, rebuilt independently

board pdata free xdata free flash saved
3dr1060 / hb1060 / ism01a 65 → 82 13 → 62 2761
hm_trp / rf50 / rfd900 66 → 83 31 → 80 2761
rfd900a 66 → 83 18 → 67 2761
mro900, rfd900p/pe, rfd900u/ue 5 → 83 4446+ → 4556+ 2342

The absolutes agree with your table exactly on the 1060 group (pdata 191 → 174, free RAM 13 → 62) and sit 4 bytes tighter elsewhere — sdcc 4.5.0 against your 4.2.0. Every delta is identical on every board: −17 pdata and −49 bytes of xdata end on the 4 KB boards, −78 pdata on the 8 KB ones, 2761 and 2342 bytes of flash. Worth noting how tight master already is on this toolchain: the five CPU_SI1030 boards have 5 bytes of the 256-byte pdata page left, which is the same page #97 overflows at 259/256 — so this gives that one 78 bytes of headroom.

ISSUE — Firmware/tools/check_code.py:62: the link-time RAM check stops running

check_xiseg() iterates map lines matching ^XISEG. rx_buf and tx_buf were the last initialised xdata objects in the firmware, so with their = {0} removed the XISEG area disappears from the map entirely — on all twelve boards, checked — and the loop body never executes. No check, no error, and no output:

master   XSEG 0x00BF 1379 bytes   XISEG 0x0622 2495 bytes   XINIT 0x9FBE 2495 bytes (code)
head     XSEG 0x00AE 3842 bytes   (no XISEG, no XINIT)

$ ./tools/check_code.py radio~hm_trp 4096     # master
   PAGED EXT. RAM   0x0001   0x00be     190      256
   EXTERNAL RAM     0x00bf   0x0fe0    3874     4096
XISEG radio~hm_trp.map - 31 bytes available
Code check OK

$ ./tools/check_code.py radio~hm_trp 4096     # this PR
Code check OK

Not a safety hole, and I checked rather than assumed: every rules_*.mk passes --xram-size $(XRAM_SIZE) to the linker, and planting a 400-byte __xdata array at your head still fails with ?ASlink-Error-Insufficient EXTERNAL RAM memory. What is lost is the secondary check and, more usefully, the N bytes available line and the memory summary the build log has always printed — on a firmware where headroom is the binding constraint, and in a PR whose entire subject is headroom, that is a real regression in visibility. One line fixes it: match ^(XISEG|XSEG) and take the larger end address, which also covers a future change that empties XSEG instead. (Your description already notes that nothing checks the pdata page at link time; this is the neighbouring gap it opens.)

Notes

  • Firmware/radio/at.h:78 — the AT command limit drops 69 → 16 on the five CPU_SI1030 boards, and an over-long command drops the radio out of AT mode. INCLUDE_AES is never defined to the compiler today (include/rules.mk:86-87 only adds an include directory, board.h:110,114 has it commented out), so the shorter limit is what mro900, rfd900p/pe and rfd900u/ue actually build with. Nothing overflows — at_input() bounds the write and handle_at_command() (tdm.c:470) rejects a remote command with len > AT_CMD_MAXLEN. But the local path is not a silent truncation: on the first excess character at.c:108-115 sets at_mode_active = 0 and at_cmd_len = 0, deliberately abandoning AT mode and returning to passthrough. No in-tree non-AES command comes close — the longest are AT&UPDATE and AT&T=RSSI at 9 characters, and ATS15=4294967295 at exactly 16 — and the 69 was sized for AT&E= plus a 64-character key, which is itself inside #ifdef INCLUDE_AES. Worth one line in the description all the same, because it is a user-visible limit change on shipped RFD900 hardware.
  • Firmware/radio/printfl.c:170 — (unsigned long)(-val) is undefined for LONG_MIN. It does the intended thing on SDCC, and the old __ltoa() negated the same way, so it is not a regression — but uval = 0UL - (unsigned long)val; is free and defined.

Checked and clean

  • The 12-byte buffer is exact, not lucky. %x and %o both set unsigned_flag (printfl.c:118-133), so decimal is the only signed radix and a negative octal cannot occur. Worst cases including the NUL: unsigned octal 37777777777 = 12, signed decimal -2147483648 = 12, unsigned decimal 11, hex 9. The backwards fill lands exactly on buffer[0] in the two 12-byte cases, and uval == 0 still prints 0 because the loop is a do/while.
  • Uppercase hex matches what __ltoa produced, so %x output is unchanged — settled by extracting __ltoa.rel from /usr/share/sdcc/lib/large/libsdcc.lib and reading the digit code: MOV A,#0x30 / ADD A,Rn, then a conditional ADD A,#0x07, i.e. '0' + digit + 7 = 'A' for digit 10.
  • Dropping = {0} cannot change behaviour: both ring buffers are written before they are read, their indices are reset in serial_init() (serial.c:208-213), and SDCC's startup clears XSEG regardless.
  • remote_at_cmd __pdata → __xdata is safe: its only uses are memcpy() and strlen() (tdm.c:459, 743-746), and it keeps the same AT_CMD_MAXLEN + 1 size as its only source.
  • The #ifdef INCLUDE_AES moves are consistent: encrypt_buff_start/end and encrypt_insert/remove have no use outside an AES guard, and ENCRYPT_BUFF_MAX stays defined exactly where encrypt_buf is.

Both a primary read and two independent second passes were run; the check_code.py point came from the cold pass as UNCONFIRMED and was then confirmed by building both revisions, which is what moves this off APPROVE. The at_input() behaviour and the printfl.c line number above are corrections the validation pass made to my first draft. Please push back on anything that looks wrong.

-val is undefined when val is the most negative long. It does the right
thing on sdcc, and the __ltoa() this replaced negated the same way, but
the unsigned form costs nothing.
check_code.py looks for the XISEG area, which holds initialised xdata. A
firmware without any is perfectly normal, and then the area is missing from
the map, the loop body never runs, and the check silently passes without
even printing the memory summary the build log has always shown.

Look at XSEG as well and take whichever area ends higher.
@julianoes

Copy link
Copy Markdown
Collaborator Author

Thanks, and thanks for rebuilding every board independently.

check_code.py stopped checking. You are right, and it is a fair catch: with no initialised xdata left there is no XISEG area in the map, so the loop never ran and the build log lost its memory summary. Fixed in a new commit: the check now looks at XSEG as well and takes whichever area ends higher, so it also survives a future change that empties XSEG instead. tests/test_build.sh prints XSEG ... N bytes available and Code check OK for all twelve boards again.

(unsigned long)(-val) for LONG_MIN. Also fixed, 0UL - (unsigned long)val.

The AT command limit on the CPU_SI1030 boards. Added to the description, including the part I had not spelled out: an over-long local command does not truncate, it drops the radio out of AT mode back into passthrough (at.c:108). The longest commands in the tree are 9 characters, and ATS15=4294967295 is exactly 16.

One note from your numbers that matters for the order of these PRs: on sdcc 4.5.0 master leaves 5 bytes of the pdata page on those boards, and #97 needs 8, so #97 wants this PR in front of it. I will rebase #97 onto master once this lands, rather than stacking them.

@AP-Review

AP-Review commented Sep 20, 2026 •

Copy link
Copy Markdown

Automated review note — AI-generated (Claude), cross-checked by an independent Codex pass against the live diff. Please sanity-check before acting.
Verdict: ACCEPT

Follow-up report: https://uav.tridgell.net/DevCallReviews/followups/2026_09_21_0847/devcall_pr_reviews.html#prSiK-99 · Also refreshed in the AIReview report

Re-reviewed at head 02ed5c1910 (previously f97dea41b3) — my earlier comment above is superseded.

Verdict: APPROVE — all three points from last round are addressed

Moving this off COMMENT. I rebuilt all twelve boards at three revisions rather than reading the diff for it — sdcc 4.5.0 #15242, tests/test_build.sh rc=0 at each.

The check_code.py regression is reproduced and then confirmed cured. This is the one that mattered, so I checked it from both ends:

revision "bytes available" lines
master 303ae63042 12 (all XISEG)
told head f97dea41b3 0 — the bug, reproduced exactly
this head 02ed5c1910 12 again, now reporting XSEG

Restored numbers are 62/62/80/62/4614/80/80/67/4610/4610/4617/4617, matching your table. The error path works too: doctoring hm_trp's XSEG size to 0x1000 gives ERROR: XSEG overflow 4270 and exit 1, and the linker guard still fires independently (a 200-byte __xdata probe gives ?ASlink-Error-Insufficient EXTERNAL RAM memory.). No false positives on bootloaders — every map has a trailing XSEG = 0x0001 base-address line and the = blocks the \w+ group, so they stay silent as before.

printfl.c LONG_MIN — applied as offered at :171, and it is better than free: 8 more bytes of flash on every one of the twelve boards (told head vs this head, identical −8 everywhere). Total saving vs master is now 2769 bytes on the 4 KB boards and 2350 on the 8 KB ones. RAM figures are byte-for-byte identical between the two heads, so nothing in your table moves.

AT_CMD_MAXLEN — documented in the description, which is what I asked for. Code correctly unchanged.

A correction I owe you

My previous table gave the 8 KB boards' xdata free as "4446+ → 4556+". That head figure was wrong. The measured values are 4610 / 4614 / 4617 — a gain of 164 bytes, not 110. The 164 is 79 bytes of the xdata high-water mark moving down because PSEG shrank (pdata lives inside the xram address space on these parts) plus 85 bytes of actual xdata content. The master column was right, as were all the 4 KB numbers. Sorry — that one was mine, and no action is needed from you.

Two notes, neither asking for a change

  • The description's "Noted while looking" bullet still says tools/check_code.py only checks XISEG — 02ed5c1 in this same PR makes that false. The rest of the bullet (nothing checks the pdata page at link time) is still accurate and worth keeping.
  • check_code.py:84 uses end >= xram_size where end is the exclusive top address, so a build ending exactly at the top of XRAM reports 0 available and errors, while the linker accepts it. Pre-existing, unchanged by you, and it errs on the safe side on a firmware this tight — leave it. I mention it only because the line moved in this commit and a later reader might take the >= for deliberate. If it is ever changed, the invariant to keep is "available = xram_size - end, and zero available is legal".

An independent cold Codex review — diff and head checkout only, no sight of any of the above — also returned APPROVE, having modelled the formatter over 262,144 16-bit cases plus the 32-bit boundaries and exercised the RAM-check function against synthetic XSEG-only, XISEG-only, combined and overflow maps.

@julianoes

Copy link
Copy Markdown
Collaborator Author

@tridge any objections against getting this one in as a first step?

@tridge
tridge merged commit ae613be into ArduPilot:master Oct 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants