fix(device-image): honor X-Draw-Pending to skip rotation during recovery
CI / test (push) Has been cancelled
CI / test (push) Has been cancelled
When the firmware sends X-Draw-Pending: 1, its drawNeeded NVS flag survived a power-loss-during-draw — it has the bytes for the previous image in its cached /img.bin and just needs another chance to finish painting them. Return the device's current image (no rotation advance), which lands as a 304 since the device claims the same image-id. Crucially this overrides the X-Boot-Reason: cold force-resync. The typical mid-draw-interruption cause IS a reset that turns the next wake into a cold boot, so without this override force-resync chases a fresh image every interruption and the device cycles through the rotation leaving torn frames on the 13.3 panel. Locked image still wins (user intent overrides recovery). Old firmware that doesn't send the header is unaffected — branch is gated on the header being present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -170,12 +170,24 @@ class DeviceImageController extends AbstractController
|
||||
// 3. Schedule says due (the normal case).
|
||||
//
|
||||
// Timer wakes after first-image otherwise stay schedule-gated.
|
||||
//
|
||||
// Recovery override: X-Draw-Pending: 1 means the device's drawNeeded
|
||||
// NVS flag survived a power-loss-during-draw. We give it back its
|
||||
// own current image so the firmware can finish repainting from its
|
||||
// cached /img.bin. This explicitly overrides the cold-boot
|
||||
// force-resync, because the typical interrupted-draw cause IS a
|
||||
// reset that turns the next wake into a cold boot — without this
|
||||
// bypass, force-resync chases a fresh image every interruption and
|
||||
// the device churns through the rotation leaving torn frames.
|
||||
$bootReason = strtolower((string) $request->headers->get('X-Boot-Reason', ''));
|
||||
$forceResync = ($bootReason === 'cold');
|
||||
$wantsBootstrap = $currentImageId < 0;
|
||||
$drawPending = $request->headers->get('X-Draw-Pending') === '1';
|
||||
|
||||
if ($device->getLockedImage() !== null) {
|
||||
$image = $device->getLockedImage();
|
||||
} elseif ($drawPending) {
|
||||
$image = $device->getCurrentImage();
|
||||
} elseif ($forceResync || $wantsBootstrap || $this->rotation->isDue($device)) {
|
||||
$image = $this->rotation->advance($device);
|
||||
} else {
|
||||
|
||||
@@ -437,6 +437,97 @@ class DeviceImageControllerTest extends AppWebTestCase
|
||||
);
|
||||
}
|
||||
|
||||
// RECOVERY HANDSHAKE: X-Draw-Pending: 1 says the device is mid-recovery
|
||||
// from a power-loss-during-draw. Even on a cold boot, the server MUST
|
||||
// suppress rotation advancement and return the device's current image,
|
||||
// so the firmware can repaint from its cached /img.bin instead of
|
||||
// chasing a fresh image every reset. Without this override, force-
|
||||
// resync defeats the firmware's drawNeeded recovery branch and the
|
||||
// device churns through the pool leaving torn frames on the panel.
|
||||
public function test_draw_pending_overrides_cold_boot_force_resync(): void
|
||||
{
|
||||
$setup = $this->createTestSetup();
|
||||
$device = $setup['device'];
|
||||
|
||||
// Land a first image so the device has a current_image set.
|
||||
$this->client->request('GET', '/api/device/' . self::MAC . '/image');
|
||||
$this->assertResponseStatusCodeSame(200);
|
||||
$imageId = $this->client->getResponse()->headers->get('X-Image-Id');
|
||||
|
||||
// Sanity-check the no-pending path: a cold boot at this point WILL
|
||||
// force-resync (this is the very behavior the pending header overrides).
|
||||
$beforeColdNoPending = $this->countHistory($device);
|
||||
$this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [
|
||||
'HTTP_X_Boot_Reason' => 'cold',
|
||||
'HTTP_X_Current_Image_Id' => $imageId,
|
||||
]);
|
||||
$afterColdNoPending = $this->countHistory($device);
|
||||
$this->assertSame(
|
||||
$beforeColdNoPending + 1,
|
||||
$afterColdNoPending,
|
||||
'sanity: bare cold-boot must force-resync (baseline for the override below)',
|
||||
);
|
||||
|
||||
// The actual feature: cold boot + X-Draw-Pending must NOT advance.
|
||||
// Rotation advancement is what writes device_image_history rows, so
|
||||
// an unchanged count is the proof that advance() was skipped.
|
||||
$imageIdAfterFirstCold = $this->client->getResponse()->headers->get('X-Image-Id');
|
||||
$beforePending = $this->countHistory($device);
|
||||
$this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [
|
||||
'HTTP_X_Boot_Reason' => 'cold',
|
||||
'HTTP_X_Current_Image_Id' => $imageIdAfterFirstCold,
|
||||
'HTTP_X_Draw_Pending' => '1',
|
||||
]);
|
||||
$afterPending = $this->countHistory($device);
|
||||
|
||||
$this->assertSame(
|
||||
$beforePending,
|
||||
$afterPending,
|
||||
'X-Draw-Pending must override cold-boot force-resync — no advance, no new history row',
|
||||
);
|
||||
$this->assertResponseStatusCodeSame(
|
||||
304,
|
||||
'with draw-pending, the device gets its current image back as 304 (it already has the bytes)',
|
||||
);
|
||||
}
|
||||
|
||||
// RECOVERY HANDSHAKE: X-Draw-Pending must also suppress the normal
|
||||
// schedule-based rotation. The device is asking to redraw what it has,
|
||||
// not for a fresh image — regardless of whether the schedule is due.
|
||||
public function test_draw_pending_overrides_due_schedule(): void
|
||||
{
|
||||
$setup = $this->createTestSetup();
|
||||
$device = $setup['device'];
|
||||
|
||||
// Establish current image, then make the schedule "always due"
|
||||
// (rotation interval of 0 means every poll is past-due).
|
||||
$this->client->request('GET', '/api/device/' . self::MAC . '/image');
|
||||
$this->assertResponseStatusCodeSame(200);
|
||||
$imageId = $this->client->getResponse()->headers->get('X-Image-Id');
|
||||
$device->setRotationIntervalMinutes(1);
|
||||
$this->em()->flush();
|
||||
// Backdate the served_at so isDue() reports true on the next poll.
|
||||
$this->em()->getConnection()->executeStatement(
|
||||
'UPDATE device_image_history SET served_at = served_at - INTERVAL \'1 hour\' WHERE device_id = :id',
|
||||
['id' => $device->getId()],
|
||||
);
|
||||
|
||||
$before = $this->countHistory($device);
|
||||
$this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [
|
||||
'HTTP_X_Boot_Reason' => 'timer',
|
||||
'HTTP_X_Current_Image_Id' => $imageId,
|
||||
'HTTP_X_Draw_Pending' => '1',
|
||||
]);
|
||||
$after = $this->countHistory($device);
|
||||
|
||||
$this->assertSame(
|
||||
$before,
|
||||
$after,
|
||||
'draw-pending must skip rotation even when the schedule reports due — no advance, no new history row',
|
||||
);
|
||||
$this->assertResponseStatusCodeSame(304);
|
||||
}
|
||||
|
||||
private function countHistory(\App\Entity\Device $device): int
|
||||
{
|
||||
return (int) $this->em()
|
||||
|
||||
Reference in New Issue
Block a user