15 Firmware Architecture Anti-Patterns That Make Embedded Systems Unmaintainable

Article 02 built an architecture for an industrial controller. Task table, memory map, ownership matrix, dependency rules, error propagation. It was clean.

Now run it forward three years. Four engineers, two of whom have left. Eleven firmware releases. Two hardware revisions. One customer escalation that produced a fix at 2 a.m. and a commit message that says “temp workaround.”

That firmware still works. It’s also become expensive to change, and nobody can point at the commit where that happened, because there isn’t one.

Firmware doesn’t rot from one bad decision. It rots from fifteen small ones that each looked correct on the day.

What follows is those fifteen. Not as a catalogue — you’ve read catalogues. Each one gets the thing that’s actually missing from most writing on this topic: the measurable tell. A grep, a runtime stat, or a scope measurement that tells you it’s happening now, while the fix is still cheap.


What Makes Something an Anti-Pattern

Not a bug. A bug is wrong on the day you write it.

An anti-pattern is correct on the day you write it and wrong eighteen months later, which is why code review doesn’t catch it. Nobody rejects a pull request for adding one global. The tenth one is the problem, and by then no single commit is at fault.

Use article 01’s frame: architecture is the set of decisions that are expensive to reverse. An anti-pattern is a decision that quietly becomes architecture. It starts reversible and stops being reversible while you’re not looking.

Which is why every entry below has a tell. Discipline doesn’t scale across four engineers and three years. Measurement does.


The Fifteen, At a Glance

#Anti-patternThe tellCost nowCost at year three
1God TaskOne task > 50% of runtime statsHoursWeeks
2God ModuleFile > 800 lines, > 3 noun prefixesHoursWeeks
3Global variable architecture.bss symbol count rising release over releaseHoursMonths
4Register access everywhereVendor headers in application/MinutesWeeks
5Fat ISRWorst-case ISR > 10 µs on a scopeHoursDays
6Blocking driverswhile (! with no timeout in drivers/MinutesDays
7Unbounded allocationmalloc in the map file; heap watermark driftingHoursMonths
8Circular dependenciesHeader graph fails tsortMinutesWeeks
9Ownership erosion> 1 call site per peripheral handleMinutesWeeks
10Swallowed errors(void) casts on result_t returnsMinutesMonths
11Magic delaysHAL_Delay outside initMinutesDays
12Hardware/application couplingApplication won’t build for the hostDaysMonths
13Reset as the only recoveryNVIC_SystemReset() call site countHoursMonths
14Logging as an afterthoughtCan’t explain a specific field resetDaysPermanent
15Architecture by existing codeFeatures touching > 3 modules, trending upTerminal

Fifteen is a lot to hold. They group into three acts.


Act One: Prototype Code That Got Promoted

Nobody decided these. The bring-up code shipped.

1. The God Task

Article 02’s task table had nine tasks, each with a deadline and a blocking policy. Here’s what it looks like in year three:

void ApplicationTask(void *arg)
{
    for (;;) {
        read_sensors();
        process_uart();
        process_network();
        update_display();
        check_configuration();
        write_logs();
        control_output();
        vTaskDelay(pdMS_TO_TICKS(10));
    }
}

It didn’t start there. It started as the control task, and each addition was one line in a review that already looked fine.

Why it fails specifically: it merges failure domains. Article 02 spent effort making sure a stuck socket couldn’t starve the control loop. Put both in one task and that guarantee is gone — not degraded, gone. Network processing that occasionally takes 50 ms is now 50 ms of control loop jitter, against a 50 µs budget. The network code isn’t the bug. The coupling is.

The tell. Turn on configGENERATE_RUN_TIME_STATS and read uxTaskGetSystemState() on a running unit. If one task holds more than half the runtime, or if its uxTaskGetStackHighWaterMark is three times deeper than the next task’s, that task is absorbing responsibilities. Track the number monthly. It only ever goes one direction.

A second, cheaper tell: count distinct #includes in that task’s translation unit. Nine tasks in article 02, each needing two or three headers. A file pulling in fourteen isn’t a task, it’s a program.

The fix, and this matters: don’t split by module. Split by the four things that actually justify a separate task — a distinct deadline, a distinct blocking behaviour, a distinct failure domain, or a distinct resource it owns. “One task per driver” produces forty tasks and a different problem. Article 05 goes into how to make those cuts properly.

2. The God Module

Same disease, different axis. device_manager.c starts at 200 lines and ends at 2,400:

device_init();          device_configure();
device_read_sensor();   device_send_packet();
device_save_config();   device_process_command();
device_log_error();     device_update_state();
device_handle_fault();

It happens because the reasoning is sound each time: this is about the device, so it goes in the device file.

The tell. Look at the prefixes. device_read_sensor and device_send_packet and device_save_config are three different nouns — sensing, comms, storage — wearing one prefix. More than three distinct nouns in a module’s public API and the module has more than one job.

Then the sentence test: can you state what this module does in one sentence with no “and” in it? device_manager.c can’t. Article 02’s spi_bus.c can — it owns SPI2 and serialises transactions on it. That’s the whole thing.

The fix: split along the layer boundaries that already exist. Device Driver, Device Service, Config Service, Comms Service, Diagnostics. The boundaries were in the architecture from day one. The module just stopped respecting them.

3. Global Variable Architecture

uint8_t  system_state;
uint32_t error_flags;
uint16_t sensor_value;
bool     device_ready;

and extern declarations for all of them in a dozen files.

Globals aren’t automatically wrong. Article 02 has plenty of static state. The problem is uncontrolled mutable state — mutable by anyone, owned by nobody. Ask the four questions and watch it fall apart: who’s allowed to write system_state? Who clears an error flag once it’s set? What happens when the ADC ISR writes sensor_value while the control task reads it? What does device_ready mean when a sensor is dead but Ethernet is fine?

Nobody knows, because there was never an owner. Article 02’s Device Manager exists to answer exactly that question, and every global that bypasses it is a hole in the answer.

The tell. This is my favourite because it takes ten seconds and it trends:

arm-none-eabi-nm --print-size build/firmware.elf \
  | grep -E ' [BbDd] ' | wc -l

Run it on every release build and plot it. The line only goes up, and the slope tells you how fast ownership is eroding. Put the number in CI output. Once it’s visible, people stop adding to it — I’ve watched that happen, and it works better than any coding standard.

The fix: an accessor and an owner.

system_state_t system_get_state(void);
result_t       system_request_state(system_state_t s);  /* owner decides */

Note it’s request, not set. Same distinction as article 02’s Device Manager. set means every caller is an owner.

4. Register Access Everywhere

GPIOA->ODR |= GPIO_PIN_5;
USART1->CR1 |= USART_CR1_TE;

in application code. This is bring-up code that never got promoted out of the prototype and into a driver.

To be clear about the nuance, because this gets overstated: register access is correct in a driver. That’s what a driver is. The failure is hardware knowledge appearing above the driver layer, where the application now knows MCU family, register names, pin mapping and bit definitions.

The tell is the CI check from article 02, and it’s the highest-value twenty lines in your build:

grep -rn "stm32h7xx" src/application/ src/services/ && exit 1

The cost, concretely. Rev C moves the status LED from PA5 to PC7. In the layered version that’s one line in the BSP. In this version, it’s a grep across the application, a re-review of every file you touched, and a regression run on product behaviour — because you edited product behaviour source to accommodate a schematic change. Article 04 is where the layer boundaries get defined properly.

11. Magic Delays

Out of numerical order because it belongs in this act — it’s the purest example of bring-up code becoming architecture.

reset_device();
HAL_Delay(100);
start_device();

Someone found that 100 ms worked. It shipped. Nobody knows if the real number is 40 or 95, and the comment, if there is one, says /* wait for device */.

Why it fails: the delay is a guess about a physical process, and the physical process depends on temperature, supply rise time, clock trim, the device’s own firmware version, and manufacturing spread. Your 100 ms has margin at 25 °C on a bench unit. At −20 °C on the slowest part in the lot, it might not.

The second failure is worse: it’s not doing nothing for 100 ms, it’s doing nothing in a task that has a deadline. Every magic delay is a hidden blocking call in your timing analysis.

The tell:

grep -rn "HAL_Delay(\|delay_ms(" src/ --include=*.c \
  | grep -v "_init\|_bringup"

Anything outside initialisation is a finding. Anything inside a task with a deadline is a defect.

The fix is a change of question. Not “how long should I wait” but “what condition tells me it’s done.” Then:

result_t device_wait_ready(uint32_t timeout_ms);

Now it returns RES_TIMEOUT when the device genuinely didn’t come up, instead of silently proceeding and failing three functions later. And if you truly can’t observe readiness — some parts give you nothing — then the delay stays, but it gets a comment with the datasheet reference, the measured worst case, and the margin you applied. A documented delay is engineering. An undocumented one is folklore.

14. Logging as an Afterthought

printf("ERROR\n");

Also out of order, also bring-up code that shipped. The printf debugging that got you through development became the diagnostic strategy by default.

The tell isn’t a grep. It’s a question, and you should ask it out loud in a review: unit 4471 in a plant in Rotterdam reset itself last Tuesday at 03:12. Why?

If the answer involves flying someone out, or asking the customer to reproduce it, you don’t have observability. You have printf.

What has to be captured, at minimum, and this is cheap if you do it at design time:

Reset reason latched from RCC->CSR in startup before anything clears it — and it must be read and cleared deliberately, because those flags accumulate across resets and a stale IWDGRSTF will send you chasing a watchdog that fired six boots ago. Boot counter. Firmware version and hardware revision. Last N structured fault records: source ID, code, timestamp, and one context word. Task stack high-water marks at the point of failure.

All of it in backup SRAM, which survives reset. Article 02 reserved 4 KB for exactly this and then never used it, which is the mistake this anti-pattern describes.

Structured events, not strings. {SRC_SPI, RES_TIMEOUT, t=41203, cs=2} is eight bytes, greppable, and answers the question. "SPI error" is nine bytes and answers nothing.

Article 11 is the full treatment. The architectural point here is smaller and more urgent: observability is a design-time decision. You cannot add it after the failure you needed it for.


Act Two: Decisions Made Under Deadline Pressure

These weren’t accidents. Someone chose them, for a reason that was good that week.

5. The Fat ISR

void UART_IRQHandler(void)
{
    char c = UART_Read();
    parser_process(c);      /* 40 µs */
    update_state();
    generate_response();
    log_event();            /* takes a mutex. from an ISR. */
    send_response();
}

The reasoning is real: the ISR already has the data, so processing it there avoids a queue and a context switch. On a bench with one interrupt source, it measures faster.

Why it fails: an ISR isn’t free-running, it’s stealing. Every microsecond in there is a microsecond stolen from whatever had a deadline, and it’s stolen at a priority the scheduler can’t arbitrate. Article 02’s control loop has a 50 µs jitter budget. A 40 µs parser in a UART ISR consumes 80% of it, and does so unpredictably, because it depends on the byte.

The log_event() line is worse than slow. It takes a mutex from interrupt context, which is undefined behaviour in FreeRTOS and will eventually hang the system in a way that reproduces once a month.

The tell, and this is a measurement, not a guess: toggle a GPIO on ISR entry and exit, run a soak under realistic traffic, and read the maximum pulse width, not the mean. Then set a budget and hold it. For article 02’s KIC-400 the number is 10 µs — one fifth of the jitter budget — and anything over it gets deferred. Pick your own number from your own tightest deadline, but pick one, write it in the coding standard, and measure against it.

Second tell, free: grep -rn "Handler" src/ -A20 | grep -v FromISR and look for any RTOS call without the FromISR suffix. That’s a defect, not a smell.

The fix is the standard split, and article 02’s fault sink is the version that doesn’t create a dependency cycle:

void UART_IRQHandler(void)
{
    uint8_t d = UART_ReadByteFromISR();
    BaseType_t woken = pdFALSE;
    xQueueSendFromISR(rx_queue, &d, &woken);
    portYIELD_FROM_ISR(woken);
}

Capture, post, return. Article 05 covers the deferral patterns in depth.

6. Blocking Drivers

while (!uart_tx_complete()) { }

No timeout. Works perfectly until the transceiver doesn’t respond, at which point that task never returns, the health monitor stops seeing it, and the watchdog resets a device that had one broken peripheral and nine working ones.

Article 02’s spi_txn_t puts timeout_ms in the struct specifically so you can’t call it without one. That’s design removing an option, which beats a review comment reminding people to be careful.

The tell:

grep -rn "while (\s*!" src/drivers/ src/middleware/

Every hit gets read. If the loop has no bounded exit, it’s a finding. And when you fix it, fix the typevoid sensor_read(data_t*) becoming result_t sensor_read(data_t*, uint32_t timeout_ms) forces every call site to acknowledge that hardware can fail to respond, which is the actual change you’re making.

7. Unbounded Allocation

malloc/free in the steady-state path.

The failure isn’t that it’s slow. It’s that it works for eleven months. Fragmentation on a long-running device with mixed allocation sizes is a slow, deterministic march toward a NULL return, and the device that fails is the one that’s been up longest — which is your best customer’s, and which you cannot reproduce on a bench that gets power-cycled daily.

Article 02’s position stands: this isn’t purity. configSUPPORT_DYNAMIC_ALLOCATION 0 everywhere with one bounded arena for mbedTLS, because mbedTLS needs an allocator and fighting it produces worse code than containing it. The requirement is that no allocation failure anywhere can reach a subsystem with a deadline.

The tell, two of them. Static: arm-none-eabi-nm build/firmware.elf | grep -w malloc — if it links, someone calls it, and you should know who. Dynamic: log the heap high-water mark hourly and look at a week of soak. Flat is fine. A slow upward drift is fragmentation, and it will reach the top eventually.


Act Three: Erosion of Structure That Was There

These are the ones article 02 designed against. Design doesn’t hold by itself.

8. Circular Dependencies

Article 02 gave the rule, the four bad edges, and the CI check. Here’s how the rule loses anyway.

It loses through logging, essentially always. Someone adds an error log inside the SPI driver. Reasonable. The logging service writes to the log store, the log store sits over the flash driver, and now:

spi_driver → logging_service → log_store → flash_driver → (shared ISR band) → spi_driver

The cycle is invisible in a code review because no single file looks wrong. It surfaces when SPI times out during a flash write, which is roughly weekly in the field and never on a bench.

The tell is mechanical, and it catches cycles the grep check misses:

gcc -MM -Isrc src/**/*.c \
  | tools/deps_to_edges.py \
  | tsort > /dev/null    # tsort exits non-zero on a cycle

Twenty minutes to wire up, runs in seconds, and it fails on the commit that introduces the cycle rather than in the field eight months later.

The fix is article 02’s rule restated as a constraint: drivers emit, they don’t call up. fault_emit(SRC_SPI, RES_TIMEOUT, cs_id) writes eight bytes to a lock-free ring with zero dependencies above it.

9. Ownership Erosion

Article 02 gave every resource exactly one owner. Erosion is always the same shape: a second call site.

Task A ──┐
Task B ──┼── SPI2
Task C ──┘

It never arrives as a decision to share the bus. It arrives as “I just need one quick read.” Then a mutex gets added, because that fixes the corruption. Then priority inheritance gets turned on, because the mutex caused an inversion. Three patches deep, and the actual problem — two owners — is now load-bearing.

The tell:

grep -rn "hspi2\|SPI2->" src/ | grep -v "drivers/spi_bus.c" | wc -l

The correct answer is zero. Any other number is the count of owners you have. Run the same grep for every peripheral handle and put the results in a table. It takes an afternoon and it will surprise you.

Symptom before you go looking: intermittent corruption on a shared bus that changes character when you add a print statement. That’s two owners, near enough every time.

10. Swallowed Errors

Article 02 built result_t with a retry policy encoded in every value, and warn_unused_result on every fallible call. Here’s how it decays.

Not void returns — those get caught. It decays through the cast:

(void)flash_write(addr, buf, len);   /* silences the warning */

and through the log-instead-of-handle:

if (flash_write(...) != RES_OK) {
    log_error("flash write failed");   /* and then continues anyway */
}

The second one is more common and more dangerous, because it looks like error handling. It isn’t. Logging is recording. Handling is deciding. A config write that failed and got logged means the device is now running with a configuration it thinks it saved and didn’t — and the failure surfaces at the next power cycle, weeks later, as corrupted config with no connection to the write that failed.

The tell:

grep -rn "(void)[a-z_]*(" src/ --include=*.c

Every hit needs a comment on the same line explaining why the result is genuinely ignorable. Article 02’s (void)sink->write(...) qualifies — it increments a drop counter first. Most don’t.

The principle, which is the one bit worth memorising: handle the error at the layer with enough context to decide. The driver knows the transfer timed out. Only the application knows whether that means retry, degrade, or stop controlling.

12. Hardware/Application Coupling

Broader than register access. The application doesn’t just touch registers, it reasons about hardware:

if (ADC1->DR > 2500) {
    GPIOB->ODR |= GPIO_PIN_3;
}

That’s a thermal protection policy expressed in ADC counts and a port bit. The engineering intent — if the temperature exceeds the limit, engage protection — isn’t recoverable from the code, and neither is the calibration that turned degrees into 2500.

The tell is the sharpest one in the article: try to build your application layer for the host.

gcc -c -Isrc/application -Isrc/services src/application/*.c

If it doesn’t compile without a cross-toolchain and vendor headers, your application is coupled to hardware. That single command is also your testability metric for article 09, and it’s why article 02 pushed a telemetry_sink_t contract downward instead of calling HAL_UART_Transmit.

The fix puts the policy above and the mapping below:

if (temperature_get_c() > OVER_TEMP_LIMIT_C) {
    thermal_protection_engage();
}

Now the ADC channel and the GPIO pin live in the BSP where rev C can move them, and the threshold is in degrees where a domain expert can review it.

13. Reset as the Only Recovery

NVIC_SystemReset() as the answer to everything.

Sometimes a reset is right. The problem is reset as the sole strategy, and the reason it’s seductive is that it works on the bench: something breaks, device reboots, device comes back. Ship it.

In the field it fails three ways. It doesn’t fix persistent causes — corrupted config, a genuinely dead sensor, a peripheral held in reset by a stuck external device — so you get a boot loop instead of a fault. It destroys the diagnostic state that would have told you the cause. And it takes down the nine working subsystems to deal with the one broken one, which is the opposite of the fault domains article 02 built.

A controller that resets on the first SPI timeout will reboot continuously in an electrically noisy panel, and each reboot loses more state than the original glitch did.

The tell: grep -rn "NVIC_SystemReset" src/ | wc -l. More than two or three call sites and reset has become a habit. Second tell, from the field: boot counter climbing on units that report no faults. That’s a device resetting its way out of problems and telling you nothing.

The fix is article 02’s escalation ladder, used one rung at a time — count, log, retry once, reset the peripheral, degrade, isolate, and only then reset the system with the reason written to backup SRAM first. Article 07 builds the recovery architecture properly.

15. Architecture by Existing Code

The last one, and the one that produces all the others.

A feature request arrives. Nobody asks what should this look like? They ask where do we put it? The answer is always the module that’s already there, so:

New feature → existing module → another patch → another dependency
            → another global → another special case

Three years of that and the architecture is a fossil record of the order features arrived in. Nobody can explain why the dependencies exist because there’s no reason — just history.

The tell, and it’s a leading indicator rather than a lagging one: track how many modules each feature touches. Feature one touched two. Feature twelve touched nine. That slope is the honest measure of architectural health, and it’s visible in your git history right now.

Second tell, softer but reliable: how often does the phrase “easier to just add it to” appear in design discussions?

The fix isn’t a rewrite. Reuse is usually right. The fix is putting one decision back in the loop:

Requirement → constraints → where does this belong? → reuse / refactor / wrap / replace

Existing code is an input to that decision. It is not the decision. That’s the whole anti-pattern in a sentence, and it’s the most expensive one on the list because it’s the one that manufactures the other fourteen.


They Reinforce Each Other

The dangerous systems don’t have one of these. They have chains, and the chain is what makes root cause impossible:

God Task → blocking driver → magic delay → missed deadline
        → watchdog reset → no diagnostics → cannot determine cause
        → add a retry to make it stop → longer stall → more resets
Globals → no ownership → race condition → intermittent failure
        → hard to debug → add a flag to work around it → more globals
HW/app coupling → rev C schematic change → large application diff
        → regression risk → add conditional code for both revs
        → coupling doubles

Notice the last step in each. The response to the symptom feeds the cause. That’s why these compound instead of accumulating, and why “we’ll clean it up next quarter” has never once happened on a project I’ve been on.


The Review, Cut Down to What Has Teeth

Long checklists don’t get used. Here are nine questions and the command that answers each. An afternoon, once a quarter.

QuestionHow you answer it
Is one task absorbing the system?uxTaskGetSystemState() runtime share
Is global state growing?nm | grep ' [BbDd] ' | wc -l, tracked per release
Does hardware knowledge leak upward?grep -rn "stm32" src/application/ src/services/
Can the application build on the host?gcc -c -Isrc/application src/application/*.c
Are ISRs within budget?GPIO toggle, max pulse width under soak
Can any operation wait forever?grep -rn "while (\s*!" src/drivers/
Does every resource have one owner?Call-site count per peripheral handle
Are errors being swallowed?grep -rn "(void)[a-z_]*("
Can you explain a specific field reset?Ask about a real unit, by serial number

Nine commands. Eight of them run in under a second. The ninth is the one that matters most and the only one you can’t automate.


Next

Several of these — register access, hardware coupling, application code reaching into drivers — are the same failure viewed from different angles: code living at the wrong layer.

Which raises the question this series has been circling since article 01. When you write a new function, what decides whether it belongs in the HAL, the BSP, a driver, or middleware? Those four words get used interchangeably in job posts and design reviews, and the confusion is why code ends up in the wrong place.

Article 04: HAL vs BSP vs Drivers vs Middleware — where firmware code actually belongs, and how to tell.


Series Index

Phase 1 — Architecture
01. Embedded Firmware Architecture Fundamentals
02. Production-Grade Firmware Architecture
03. Firmware Architecture Anti-Patterns (you are here)

Phase 2 — Implementation
04. HAL vs BSP vs Drivers vs Middleware
05. RTOS Architecture
06. State Machine Architecture

Phase 3 — Failure & Resilience
07. Error Handling & Recovery
08. Firmware Security Architecture

Phase 4 — Verification
09. Designing Firmware for Testability

Phase 5 — Product Lifecycle
10. Designing Firmware for 10-Year Products
11. Field Diagnostics & Observability
12. OTA & Safe Firmware Updates

Leave a Reply

Your email address will not be published. Required fields are marked *