mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-14 14:23:14 -06:00
BrightSign: replace the Roku APIs, and a literal that stopped the script loading
The host scripts were written against the wrong reference. BrightScript is
Roku's language, the two API references read almost identically, and nothing
here can run either — so a call to an object that does not exist looked exactly
like a call to one that does. Verified on an XT245 and against BrightSign's
published reference; every item below was confirmed, not guessed.
THE ONE THAT COST A BOOT. `body$ = "{""width"":"` is not an escaped quote —
BrightScript has no escape sequences, so that is three adjacent literals with no
operator, and the compiler rejects the WHOLE FILE:
ScriptLoadError: Syntax Error. (compile error &h02) in SSD:/autorun.brs(196)
Not a broken feature — no player at all, on a display showing nothing. Built
with Chr(34) now.
THE ONE IN THE FIELD. MatchFiles takes a DIRECTORY plus a pattern and returns
nothing when the pattern contains a separator; we passed a full path as both
arguments. FileExists() could never return true, for any file, on any player.
That is exactly what a consultant hit: "[st-autozip] no autorun.zip on any
volume" printed while `dir SD:` listed autorun.zip. It also silently disabled
the entire self-update path. (Related: `autorun.zip_invalid` on his card is not
an accusation — it is the rename BrightSign's own example performs AFTER a
successful unpack. Our STORED-only insistence fixed a problem that was never
there; deflate32 is supported.)
Roku objects that do not exist here, each of which disabled a feature quietly:
roFileSystem (~20 sites — the update path could never mark a package applied),
roMessageDigest (verification returned false unconditionally and burned an
attempt counter), PostFromStringWithRetry (a snapshot request raised "member
function not found" from inside the event loop and took the player down).
Replaced with MoveFile/DeleteFile, roHashGenerator, and an async POST on a
message port, which is the only documented way to read a POST body.
Unpack() returns Void, so `if not package.Unpack(...)` was a type error dressed
as an error check; success is now proven by looking for the extracted file. And
Unpack() DELETES everything already in its target — unpacking an update to the
volume root would have erased the player's provisioning and its whole content
pool as a side effect of a routine upgrade. It stages to a directory of its own
and moves files into place, deliberately never overwriting screentinker.json.
Also: SetMode() takes one argument (rotation belongs to SetScreenModes, which
REBOOTS, so it only fires on a real change); GetStorageStatus is unreliable with
"USBn:"; a load-error names its resource in `uri`, not `url`.
server/test/brightscript-api-surface.test.js is the cheap thing that would have
caught all of it: a deny-list of Roku APIs plus the argument shapes and literal
forms that compile and then do nothing. It cannot prove the scripts are right;
it stops these specific mistakes coming back. It has already earned its keep —
it caught a comment I had broken while writing this change.
Verified on hardware: the player loads clean from the NVMe, restores its cached
playlist and plays BEFORE the server connects, fetches media with the new
?rev= revision, and registers against alpha rc4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
ScreenTinker
parent
76ba6c7506
commit
647a2bcc56
|
|
@ -193,7 +193,13 @@ Sub TakeSnapshot(widget As Object, req As Object)
|
|||
if req <> invalid and req.width <> invalid then w% = req.width
|
||||
if req <> invalid and req.height <> invalid then h% = req.height
|
||||
|
||||
body$ = "{""width"":" + Stri(w%).Trim() + ",""height"":" + Stri(h%).Trim() + "}"
|
||||
' BrightScript has NO escape sequences in string literals: "" does not mean an escaped quote,
|
||||
' it ends one string and begins another, so `"{""width"":"` is three literals with no operator
|
||||
' between them — a compile error that stops the WHOLE SCRIPT loading, not just this function.
|
||||
' A quote has to come from Chr(34). This line is why the player booted to nothing:
|
||||
' ScriptLoadError: Syntax Error. (compile error &h02) in SSD:/autorun.brs(196)
|
||||
q$ = Chr(34)
|
||||
body$ = "{" + q$ + "width" + q$ + ":" + Stri(w%).Trim() + "," + q$ + "height" + q$ + ":" + Stri(h%).Trim() + "}"
|
||||
|
||||
ut = CreateObject("roUrlTransfer")
|
||||
if ut = invalid then
|
||||
|
|
@ -205,8 +211,28 @@ Sub TakeSnapshot(widget As Object, req As Object)
|
|||
ut.SetUserAndPassword("admin", serial$)
|
||||
ut.AddHeader("Content-Type", "application/json")
|
||||
|
||||
resp$ = ut.PostFromStringWithRetry(body$, 1)
|
||||
if resp$ = invalid or resp$ = "" then
|
||||
' PostFromStringWithRetry does not exist — calling it raised "Member function not found" from
|
||||
' inside the event loop, i.e. a snapshot request took the whole player down. And the synchronous
|
||||
' PostFromString() is no use either: it returns only a response CODE and discards the body, which
|
||||
' is where the thumbnail is. The documented way to read a POST response is asynchronous, on a
|
||||
' message port.
|
||||
port = CreateObject("roMessagePort")
|
||||
ut.SetPort(port)
|
||||
if not ut.AsyncPostFromString(body$) then
|
||||
widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "could not reach the local DWS" })
|
||||
return
|
||||
end if
|
||||
|
||||
' Bounded: a capture that never answers must not wedge the event loop that drives playback.
|
||||
ev = Wait(20000, port)
|
||||
if type(ev) <> "roUrlEvent" then
|
||||
ut.AsyncCancel()
|
||||
widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "the local DWS did not answer" })
|
||||
return
|
||||
end if
|
||||
|
||||
resp$ = ev.GetString()
|
||||
if resp$ = "" then
|
||||
widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "no response from the local DWS" })
|
||||
return
|
||||
end if
|
||||
|
|
@ -258,18 +284,48 @@ Sub SetOrientation(widget As Object, o As String)
|
|||
return
|
||||
end if
|
||||
|
||||
mode$ = vm.GetMode()
|
||||
if mode$ = invalid or mode$ = "" then mode$ = "1920x1080x60p"
|
||||
|
||||
ok = vm.SetMode(mode$, transform$)
|
||||
if ok = invalid then ok = false
|
||||
|
||||
if ok then
|
||||
print "[st] orientation "; o; " -> transform "; transform$
|
||||
else
|
||||
print "[st] orientation "; o; ": SetMode refused transform "; transform$
|
||||
' SetMode() takes ONE argument — a mode string. Passing a transform as a second argument was a
|
||||
' "wrong number of function parameters" abort, so this Sub never reached its own reply and the
|
||||
' page never learned to fall back. Rotation lives on SetScreenModes(), whose per-screen config
|
||||
' carries a `transform` of normal|90|180|270 and rotates EVERYTHING including the video plane.
|
||||
' Implemented in BOS 9.0.15+; an older player simply has no method here and is told so.
|
||||
' FindMemberFunction is the guard BrightSign's own scripts use for a method that may not exist
|
||||
' on this OS version — safer than naming a member directly, which would attempt the call.
|
||||
if FindMemberFunction(vm, "GetScreenModes") = invalid or FindMemberFunction(vm, "SetScreenModes") = invalid then
|
||||
print "[st] orientation: this OS has no SetScreenModes — the page keeps its CSS fallback"
|
||||
widget.PostJSMessage({ type: "orientation-result", ok: false, error: "SetScreenModes unavailable" })
|
||||
return
|
||||
end if
|
||||
widget.PostJSMessage({ type: "orientation-result", ok: ok, transform: transform$ })
|
||||
|
||||
configs = vm.GetScreenModes()
|
||||
if configs = invalid or configs.Count() = 0 then
|
||||
widget.PostJSMessage({ type: "orientation-result", ok: false, error: "no screen configuration" })
|
||||
return
|
||||
end if
|
||||
|
||||
' ⚠️ SetScreenModes REBOOTS the player when it changes the screen configuration. A playlist push
|
||||
' repeats the current orientation on every update, so applying it unconditionally would reboot
|
||||
' the display every time the server spoke to it. Only a real change is worth a reboot.
|
||||
changed = false
|
||||
for each c in configs
|
||||
if c.transform <> transform$ then
|
||||
c.transform = transform$
|
||||
changed = true
|
||||
end if
|
||||
end for
|
||||
|
||||
if not changed then
|
||||
print "[st] orientation already "; transform$; " — nothing to do"
|
||||
widget.PostJSMessage({ type: "orientation-result", ok: true, transform: transform$ })
|
||||
return
|
||||
end if
|
||||
|
||||
' Tell the page BEFORE the call: the reboot may take the player out mid-sentence, and a display
|
||||
' that rotates without ever confirming looks like the command was ignored.
|
||||
widget.PostJSMessage({ type: "orientation-result", ok: true, transform: transform$, rebooting: true })
|
||||
print "[st] orientation "; o; " -> transform "; transform$; " (the player will now reboot)"
|
||||
sleep(1000)
|
||||
vm.SetScreenModes(configs)
|
||||
End Sub
|
||||
|
||||
'=== capability probe =======================================================================
|
||||
|
|
@ -289,10 +345,19 @@ End Sub
|
|||
Function StorageProbe() As Object
|
||||
result = { present: false, volume: "", free_mb: 0, total_mb: 0 }
|
||||
|
||||
volumes = ["SSD:", "SD:", "USB1:"]
|
||||
' "USB:" not "USB1:" — the docs warn that GetStorageStatus() results are UNRELIABLE when called
|
||||
' with a "USBn:" parameter, and list "USB:", "SD:", "SSD:", "SD2:/", "Flash:" as the drive
|
||||
' strings it understands. One roStorageHotplug for the whole loop rather than one per volume.
|
||||
hp = CreateObject("roStorageHotplug")
|
||||
' Ask the platform which volumes exist rather than guessing; the static list is the fallback for
|
||||
' an OS without the enumerator. Same shape BrightSign's own boilerplate uses.
|
||||
volumes = ["SSD:", "SD:", "SD2:", "USB:"]
|
||||
if hp <> invalid and FindMemberFunction(hp, "GetStorages") <> invalid then
|
||||
found = hp.GetStorages()
|
||||
if found <> invalid and found.Count() > 0 then volumes = found
|
||||
end if
|
||||
for each v in volumes
|
||||
mounted = false
|
||||
hp = CreateObject("roStorageHotplug")
|
||||
if hp <> invalid then
|
||||
st = hp.GetStorageStatus(v)
|
||||
if st <> invalid and st.mounted then mounted = true
|
||||
|
|
@ -372,8 +437,16 @@ Function PackageVersion() As String
|
|||
return "0.0.0-dev" ' ST_PACKAGE_VERSION (stamped at build time — do not edit by hand)
|
||||
End Function
|
||||
|
||||
Function DoesFileExist(filePath$ As String) As Boolean
|
||||
files = MatchFiles(filePath$, filePath$)
|
||||
' Does [name] exist in directory [dir]?
|
||||
'
|
||||
' MatchFiles takes a DIRECTORY plus a pattern, and returns nothing when the pattern contains a
|
||||
' separator. The previous version passed a full path as both arguments, so it answered "no" for
|
||||
' every file on every player — which silently disabled this entire self-update path: the pending
|
||||
' package was never seen, the .part file was never cleaned up, and the .done marker was never
|
||||
' noticed. Two arguments, so there is no path-splitting to get wrong.
|
||||
Function FileExists(dir As String, name As String) As Boolean
|
||||
files = MatchFiles(dir, name)
|
||||
if files = invalid then return false
|
||||
return files.Count() > 0
|
||||
End Function
|
||||
|
||||
|
|
@ -385,33 +458,55 @@ End Function
|
|||
' storage root the OS no longer auto-processes the archive — so from then on the host has to do it
|
||||
' itself, or self-update would work exactly once.
|
||||
Sub ApplyPendingPackage(root As String)
|
||||
dir$ = root + "/"
|
||||
zipPath$ = root + "/autorun.zip"
|
||||
donePath$ = root + "/autorun.zip.done"
|
||||
badPath$ = root + "/autorun.zip.bad"
|
||||
stage$ = root + "/st-staging"
|
||||
|
||||
if not DoesFileExist(zipPath$) then return
|
||||
if DoesFileExist(donePath$) then return ' already unpacked; extracting again is the boot loop
|
||||
if not FileExists(dir$, "autorun.zip") then return
|
||||
if FileExists(dir$, "autorun.zip.done") then return ' already unpacked; again is the boot loop
|
||||
|
||||
print "[st-update] unpacking pending package"
|
||||
|
||||
package = CreateObject("roBrightPackage", zipPath$)
|
||||
if package = invalid then
|
||||
print "[st-update] ERROR: archive unreadable (is it STORED?) — parking it as .bad"
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs <> invalid then fs.Rename(zipPath$, badPath$)
|
||||
print "[st-update] ERROR: archive unreadable — parking it as .bad"
|
||||
MoveFile(zipPath$, badPath$)
|
||||
return
|
||||
end if
|
||||
|
||||
if not package.Unpack(root + "/") then
|
||||
print "[st-update] ERROR: extract failed — parking it as .bad so we do not retry forever"
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs <> invalid then fs.Rename(zipPath$, badPath$)
|
||||
' ⚠️ Unpack() DELETES everything already in its target directory: "Providing a destination path
|
||||
' of SD:/ will wipe all preexisting files from the card". Unpacking straight to the volume root
|
||||
' would therefore erase this player's provisioning and its entire content pool on every update —
|
||||
' the update would work and the display would come back empty and unpaired.
|
||||
'
|
||||
' So it goes to a staging directory of its own, and the files are moved into place afterwards.
|
||||
' The wipe is then a FEATURE: it clears any half-extracted remains of a previous attempt.
|
||||
CreateDirectory(stage$)
|
||||
package.Unpack(stage$ + "/")
|
||||
|
||||
' Unpack() returns Void, so success is proven by looking for what should now exist rather than
|
||||
' by testing a return value that was never there.
|
||||
if not FileExists(stage$ + "/", "autorun.brs") then
|
||||
print "[st-update] ERROR: extract produced no autorun.brs — parking it as .bad"
|
||||
MoveFile(zipPath$, badPath$)
|
||||
return
|
||||
end if
|
||||
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs = invalid then return
|
||||
if not fs.Rename(zipPath$, donePath$) then
|
||||
' screentinker.json is deliberately NOT copied over: it carries THIS player's provisioning
|
||||
' (server URL, device id), and the copy inside a package carries the build's defaults. Letting
|
||||
' an update overwrite it would re-point or unpair the display as a side effect of a routine
|
||||
' upgrade — silently, and on every player at once.
|
||||
moved% = 0
|
||||
for each name in MatchFiles(stage$, "*")
|
||||
if name <> "screentinker.json" then
|
||||
if MoveFile(stage$ + "/" + name, root + "/" + name) then moved% = moved% + 1
|
||||
end if
|
||||
end for
|
||||
print "[st-update] installed "; moved%; " file(s)"
|
||||
|
||||
if not MoveFile(zipPath$, donePath$) then
|
||||
' Refusing to reboot without the marker: we would extract and reboot forever.
|
||||
print "[st-update] ERROR: could not mark done — not rebooting"
|
||||
return
|
||||
|
|
@ -442,7 +537,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String)
|
|||
xfer.SetUrl(url$)
|
||||
xfer.EnablePeerVerification(true)
|
||||
body$ = xfer.GetToString()
|
||||
if body$ = "" then return ' unreachable: keep running what works
|
||||
if body$ = "" then return ' unreachable server: keep running what works
|
||||
|
||||
manifest = ParseJson(body$)
|
||||
if manifest = invalid then return
|
||||
|
|
@ -456,8 +551,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String)
|
|||
|
||||
' Any earlier partial is deleted first: resuming into an existing file would concatenate two
|
||||
' downloads into something that hashes to neither.
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs <> invalid and DoesFileExist(partPath$) then fs.Delete(partPath$)
|
||||
if FileExists(root + "/", "autorun.zip.part") then DeleteFile(partPath$)
|
||||
|
||||
dl = CreateObject("roUrlTransfer")
|
||||
if dl = invalid then return
|
||||
|
|
@ -466,7 +560,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String)
|
|||
if dl.GetToFile(partPath$) <> 200 then
|
||||
print "[st-update] download failed"
|
||||
RecordPackageAttempt(reg, attempts% + 1)
|
||||
if fs <> invalid then fs.Delete(partPath$)
|
||||
DeleteFile(partPath$)
|
||||
return
|
||||
end if
|
||||
|
||||
|
|
@ -474,15 +568,14 @@ Sub CheckPackageUpdate(cfg As Object, root As String)
|
|||
if not VerifyPackage(partPath$, manifest.sha256, manifest.size) then
|
||||
print "[st-update] VERIFICATION FAILED — discarding, staying on "; PackageVersion()
|
||||
RecordPackageAttempt(reg, attempts% + 1)
|
||||
if fs <> invalid then fs.Delete(partPath$)
|
||||
DeleteFile(partPath$)
|
||||
return
|
||||
end if
|
||||
|
||||
' Promote. Marker first — see the ordering note above.
|
||||
if fs = invalid then return
|
||||
if DoesFileExist(root + "/autorun.zip.done") then fs.Delete(root + "/autorun.zip.done")
|
||||
if DoesFileExist(root + "/autorun.zip") then fs.Delete(root + "/autorun.zip")
|
||||
if not fs.Rename(partPath$, root + "/autorun.zip") then
|
||||
if FileExists(root + "/", "autorun.zip.done") then DeleteFile(root + "/autorun.zip.done")
|
||||
if FileExists(root + "/", "autorun.zip") then DeleteFile(root + "/autorun.zip")
|
||||
if not MoveFile(partPath$, root + "/autorun.zip") then
|
||||
print "[st-update] ERROR: could not stage the package — staying put"
|
||||
RecordPackageAttempt(reg, attempts% + 1)
|
||||
return
|
||||
|
|
@ -505,35 +598,40 @@ End Sub
|
|||
' sha256 + size. Both matter: the hash proves the bytes are the ones we were promised, the size
|
||||
' floor catches an error page or captive-portal login saved under the package's name.
|
||||
Function VerifyPackage(path As String, expected As String, expectedSize As Integer) As Boolean
|
||||
if expected = invalid or expected = "" then return false
|
||||
' Guard the arguments before the type declarations do it for us: a manifest missing sha256 or
|
||||
' size passes `invalid` into an `As String`/`As Integer` parameter, which is a runtime error at
|
||||
' the CALL — before any check inside the function could help.
|
||||
if expected = "" then return false
|
||||
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs = invalid then return false
|
||||
|
||||
info = fs.Stat(path)
|
||||
if info = invalid then return false
|
||||
if info.size < 1024 then
|
||||
print "[st-update] package is implausibly small ("; info.size; " bytes)"
|
||||
return false
|
||||
end if
|
||||
if expectedSize > 0 and info.size <> expectedSize then
|
||||
print "[st-update] size mismatch: got "; info.size; " expected "; expectedSize
|
||||
' roByteArray + roHashGenerator. The previous version used roFileSystem.Stat/OpenInputFile and
|
||||
' roMessageDigest — all three are Roku objects that do not exist on BrightSign, so this function
|
||||
' returned false unconditionally and every self-update failed verification and burned an
|
||||
' attempt. The package is tens of kilobytes, so reading it whole is cheaper than the streaming
|
||||
' loop it replaces.
|
||||
ba = CreateObject("roByteArray")
|
||||
if ba = invalid then return false
|
||||
if not ba.ReadFile(path) then
|
||||
print "[st-update] package unreadable at "; path
|
||||
return false
|
||||
end if
|
||||
|
||||
digest = CreateObject("roMessageDigest")
|
||||
size% = ba.Count()
|
||||
if size% < 1024 then
|
||||
print "[st-update] package is implausibly small ("; size%; " bytes)"
|
||||
return false
|
||||
end if
|
||||
if expectedSize > 0 and size% <> expectedSize then
|
||||
print "[st-update] size mismatch: got "; size%; " expected "; expectedSize
|
||||
return false
|
||||
end if
|
||||
|
||||
hg = CreateObject("roHashGenerator", "sha256")
|
||||
if hg = invalid then return false
|
||||
digest = hg.Hash(ba)
|
||||
if digest = invalid then return false
|
||||
digest.SetAlgorithm("sha256")
|
||||
|
||||
file = fs.OpenInputFile(path)
|
||||
if file = invalid then return false
|
||||
while true
|
||||
chunk = file.Read(65536)
|
||||
if chunk.Count() = 0 then exit while
|
||||
digest.Update(chunk)
|
||||
end while
|
||||
|
||||
return LCase(digest.Final()) = LCase(expected)
|
||||
' Hash() answers with an roByteArray, not a string.
|
||||
return LCase(digest.ToHexString()) = LCase(expected)
|
||||
End Function
|
||||
|
||||
'=== main ===================================================================================
|
||||
|
|
@ -613,7 +711,10 @@ Sub Main()
|
|||
' Back off, then fall back to the local page so the screen says something
|
||||
' truthful instead of showing white. The local page keeps retrying the server.
|
||||
retries = retries + 1
|
||||
print "[st] load-error ("; retries; "): "; data.url
|
||||
' The key is `uri` on a load-error; `url` belongs to download-request. Printing
|
||||
' the wrong one meant the single diagnostic that names the failing resource always
|
||||
' printed "invalid".
|
||||
print "[st] load-error ("; retries; "): "; data.uri
|
||||
sleep(ChooseBackoff(retries))
|
||||
if retries >= 3 then
|
||||
' The server URL rides along so the fallback page can name it on screen and
|
||||
|
|
|
|||
|
|
@ -13,11 +13,13 @@
|
|||
' being processed at all. autorun.brs belongs INSIDE the zip, which is where the build script puts
|
||||
' it (scripts/build-autorun-zip.sh).
|
||||
'
|
||||
' Unpacks with roBrightPackage, which is what BrightSign's own tooling uses — NOT roUnzip.
|
||||
' A BrightSign consultant flagged this after our first archive failed his automated deployment:
|
||||
' the zip reached the player and then could not be opened. Two causes, both fixed:
|
||||
' - the archive must be STORED, no compression (scripts/build-autorun-zip.sh now asserts it)
|
||||
' - roBrightPackage is the supported reader for a player package
|
||||
' Unpacks with roBrightPackage, which is what BrightSign's own tooling uses.
|
||||
'
|
||||
' On compression: roBrightPackage supports deflate32 with default options, PPMd, and "no
|
||||
' compression". What it does NOT support is bzip2, LZMA, Deflate64 and Zip64 (the last being what
|
||||
' Windows Explorer's built-in zipper produces). We build STORED, which is stricter than required and
|
||||
' costs nothing at this size — but note that compression was NOT the cause of the deployment failure
|
||||
' this script was blamed for. That was the MatchFiles bug below.
|
||||
'
|
||||
' Requires BrightSignOS 7.0.60+.
|
||||
|
||||
|
|
@ -26,9 +28,9 @@
|
|||
' for the file beats assuming a volume: extracting to "SD:/" on a player with no card writes to a
|
||||
' volume that does not exist, and the deployment silently does nothing.
|
||||
Function SourceRoot() As String
|
||||
volumes = ["USB1:", "SD:", "SSD:", "FLASH:"]
|
||||
volumes = ["USB1:", "SD:", "SD2:", "SSD:", "FLASH:"]
|
||||
for each v in volumes
|
||||
if DoesFileExist(v + "/autorun.zip") then return v
|
||||
if FileExists(v + "/", "autorun.zip") then return v
|
||||
end for
|
||||
return ""
|
||||
End Function
|
||||
|
|
@ -45,14 +47,14 @@ Sub Main()
|
|||
|
||||
print "[st-autozip] volume "; root$
|
||||
|
||||
if not DoesFileExist(zipPath$) then
|
||||
if not FileExists(extractPath$, "autorun.zip") then
|
||||
print "[st-autozip] no autorun.zip at "; zipPath$; " — nothing to do"
|
||||
return
|
||||
end if
|
||||
|
||||
' Idempotence. Without this the player extracts, reboots, extracts again, reboots again —
|
||||
' a boot loop that looks like a hardware fault.
|
||||
if DoesFileExist(donePath$) then
|
||||
if FileExists(extractPath$, "autorun.zip.done") then
|
||||
print "[st-autozip] already unpacked (autorun.zip.done present) — leaving it alone"
|
||||
return
|
||||
end if
|
||||
|
|
@ -67,20 +69,22 @@ Sub Main()
|
|||
return
|
||||
end if
|
||||
|
||||
if not package.Unpack(extractPath$) then
|
||||
print "[st-autozip] ERROR: unpack failed"
|
||||
' Unpack() returns VOID — there is no boolean to test, and `if not package.Unpack(...)` was a
|
||||
' type error rather than an error check. Success is proven the only way that actually means
|
||||
' anything: the file we came here to install is now on the card.
|
||||
package.Unpack(extractPath$)
|
||||
|
||||
if not FileExists(extractPath$, "autorun.brs") then
|
||||
print "[st-autozip] ERROR: unpack produced no autorun.brs — leaving the archive for a retry"
|
||||
return
|
||||
end if
|
||||
|
||||
print "[st-autozip] extracted"
|
||||
|
||||
fs = CreateObject("roFileSystem")
|
||||
if fs = invalid then
|
||||
print "[st-autozip] ERROR: no roFileSystem — cannot mark the archive done"
|
||||
return
|
||||
end if
|
||||
|
||||
if not fs.Rename(zipPath$, donePath$) then
|
||||
' MoveFile/DeleteFile are GLOBAL functions on BrightSign. roFileSystem is a Roku object and does
|
||||
' not exist here, so every one of these calls used to return invalid — which meant the archive
|
||||
' was never marked done and the player never rebooted into the player it had just installed.
|
||||
if not MoveFile(zipPath$, donePath$) then
|
||||
print "[st-autozip] ERROR: could not rename the archive; refusing to reboot into a loop"
|
||||
return
|
||||
end if
|
||||
|
|
@ -90,7 +94,17 @@ Sub Main()
|
|||
RebootSystem()
|
||||
End Sub
|
||||
|
||||
Function DoesFileExist(filePath$ As String) As Boolean
|
||||
files = MatchFiles(filePath$, filePath$)
|
||||
' Does [name] exist in directory [dir]?
|
||||
'
|
||||
' THE BUG THIS FIXES. The previous version passed a full path as BOTH arguments of MatchFiles.
|
||||
' MatchFiles takes a DIRECTORY plus a pattern, and the documentation is explicit: "you will get no
|
||||
' results if the pattern contains a directory separator". So it returned an empty list every time,
|
||||
' for every file, on every player — and this script reported "no autorun.zip on any volume" while a
|
||||
' `dir SD:` sat there listing autorun.zip. Reported from a real deployment on an HD1026.
|
||||
'
|
||||
' Two arguments rather than one path, so there is no string-splitting to get wrong.
|
||||
Function FileExists(dir As String, name As String) As Boolean
|
||||
files = MatchFiles(dir, name)
|
||||
if files = invalid then return false
|
||||
return files.Count() > 0
|
||||
End Function
|
||||
|
|
|
|||
200
server/test/brightscript-api-surface.test.js
Normal file
200
server/test/brightscript-api-surface.test.js
Normal file
|
|
@ -0,0 +1,200 @@
|
|||
'use strict';
|
||||
|
||||
// BrightScript cannot be run, linted or type-checked here — the only interpreter is a player. So a
|
||||
// call to an object that does not exist looks exactly like a call to one that does, right up until
|
||||
// a display in the field stops working.
|
||||
//
|
||||
// That is not hypothetical. `brightsign/*.brs` shipped with a whole family of ROKU APIs in it —
|
||||
// roFileSystem, roMessageDigest, PostFromStringWithRetry — because BrightScript is Roku's language
|
||||
// and the two references read almost identically. Each one silently disabled a feature: the
|
||||
// self-update path could never mark a package applied, verification returned false unconditionally
|
||||
// and burned an attempt counter, and a snapshot request raised "Member function not found" from
|
||||
// inside the event loop, taking the player down. None of it was visible from here.
|
||||
//
|
||||
// This is the cheapest thing that would have caught all of it: a deny-list of APIs that exist on
|
||||
// Roku and not on BrightSign, plus the argument-shape mistakes that made calls compile and then do
|
||||
// nothing. It cannot prove the scripts are right. It does stop these specific, expensive mistakes
|
||||
// coming back — and every entry below was paid for once already.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const DIR = path.join(__dirname, '..', '..', 'brightsign');
|
||||
const FILES = fs.readdirSync(DIR).filter((f) => f.endsWith('.brs'));
|
||||
|
||||
/** Source with comment lines stripped, so prose about a bug is not mistaken for the bug. */
|
||||
function code(file) {
|
||||
return fs.readFileSync(path.join(DIR, file), 'utf8')
|
||||
.split('\n')
|
||||
.filter((l) => !/^\s*'/.test(l))
|
||||
.join('\n');
|
||||
}
|
||||
|
||||
test('there are BrightScript files to check', () => {
|
||||
assert.ok(FILES.length >= 2, `expected the host scripts, found ${FILES.join(', ')}`);
|
||||
});
|
||||
|
||||
// Objects that exist on Roku and NOT on BrightSign. Verified against BrightSign's Object Reference,
|
||||
// which lists every ro* object the platform has.
|
||||
const ROKU_ONLY = [
|
||||
['roFileSystem', 'use the global MoveFile / DeleteFile / CopyFile, or roByteArray to read'],
|
||||
['roMessageDigest', 'use roHashGenerator("sha256"); Hash() answers with an roByteArray'],
|
||||
['roUnzip', 'use roBrightPackage — roUnzip is not the reader for a player package'],
|
||||
['roRegistryKey', 'use roRegistrySection'],
|
||||
['roAssociativeArrayEx', 'plain roAssociativeArray'],
|
||||
];
|
||||
|
||||
for (const [obj, advice] of ROKU_ONLY) {
|
||||
test(`no ${obj} — it does not exist on BrightSign (${advice})`, () => {
|
||||
for (const f of FILES) {
|
||||
assert.ok(!code(f).includes(obj), `${f} calls ${obj}, which is a Roku object. ${advice}`);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Methods that do not exist on the object they are called on.
|
||||
const BAD_METHODS = [
|
||||
['PostFromStringWithRetry', 'roUrlTransfer has no retry variant; AsyncPostFromString + roUrlEvent is how a POST body is read'],
|
||||
['.Final(', 'roMessageDigest-era API; roHashGenerator returns the digest from Hash()'],
|
||||
['OpenInputFile', 'no roFileSystem here; read with roByteArray.ReadFile'],
|
||||
];
|
||||
|
||||
for (const [needle, advice] of BAD_METHODS) {
|
||||
test(`no ${needle.replace(/[.(]/g, '')} — ${advice}`, () => {
|
||||
for (const f of FILES) {
|
||||
assert.ok(!code(f).includes(needle), `${f} calls ${needle}. ${advice}`);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test('MatchFiles is called with a DIRECTORY and a bare pattern', () => {
|
||||
// The bug that made a deployment report an empty card while `dir SD:` listed the file. MatchFiles
|
||||
// takes a directory plus a pattern and is documented to return nothing when the pattern contains
|
||||
// a separator — so passing a path as the second argument answers "no" for every file that exists.
|
||||
for (const f of FILES) {
|
||||
for (const m of code(f).matchAll(/MatchFiles\(([^)]*)\)/g)) {
|
||||
const args = m[1].split(',').map((a) => a.trim());
|
||||
assert.equal(args.length, 2, `${f}: MatchFiles needs exactly two arguments, got ${m[0]}`);
|
||||
assert.ok(!/["'].*\/.*["']/.test(args[1]) && !/\+/.test(args[1]),
|
||||
`${f}: the MatchFiles PATTERN must not contain a path separator — ${m[0]}`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('Unpack() is never used as if it returned a boolean', () => {
|
||||
// Unpack(path) is declared As Void. `if not package.Unpack(...)` is a type error, not an error
|
||||
// check — and reads exactly like one. Success is proven by looking for an expected file instead.
|
||||
for (const f of FILES) {
|
||||
const src = code(f);
|
||||
assert.ok(!/if\s+not\s+\w*\.?Unpack\s*\(/i.test(src),
|
||||
`${f}: Unpack() returns Void — test for an extracted file rather than its return value`);
|
||||
assert.ok(!/=\s*\w*\.?Unpack\s*\(/i.test(src), `${f}: Unpack() returns nothing to assign`);
|
||||
}
|
||||
});
|
||||
|
||||
test('Unpack() never targets a volume root', () => {
|
||||
// "Providing a destination path of SD:/ will wipe all preexisting files from the card." Unpacking
|
||||
// an update straight to the root would erase the player's provisioning and its whole content pool
|
||||
// as a side effect of a routine upgrade.
|
||||
for (const f of FILES) {
|
||||
for (const m of code(f).matchAll(/\.Unpack\(([^)]*)\)/g)) {
|
||||
const arg = m[1].trim();
|
||||
assert.ok(!/^(root|root\$)\s*\+\s*"\/"$/.test(arg) && !/^"[A-Z0-9]+:\/"$/.test(arg),
|
||||
`${f}: ${m[0]} unpacks to a volume root, which DELETES everything already there. Stage it.`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('roVideoMode.SetMode() is called with exactly one argument', () => {
|
||||
// SetMode(mode As String). A second argument is a "wrong number of function parameters" abort.
|
||||
// Rotation belongs to SetScreenModes(), whose config carries the transform.
|
||||
for (const f of FILES) {
|
||||
for (const m of code(f).matchAll(/\.SetMode\(([^)]*)\)/g)) {
|
||||
assert.equal(m[1].split(',').length, 1, `${f}: ${m[0]} — SetMode takes one argument`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('GetStorageStatus is not called with a USBn: drive string', () => {
|
||||
// Documented: "The results of the GetStorageStatus() method are unreliable when called with a
|
||||
// USBn: parameter." The drive strings it understands are "USB:", "SD:", "SSD:", "SD2:/", "Flash:".
|
||||
for (const f of FILES) {
|
||||
assert.ok(!/GetStorageStatus\(\s*"USB\d/i.test(code(f)),
|
||||
`${f}: GetStorageStatus is unreliable with USBn: — use "USB:"`);
|
||||
}
|
||||
});
|
||||
|
||||
test('a load-error is reported with its uri, not a url', () => {
|
||||
// `url` is a key of download-request; a load-error carries `uri`. Reading the wrong one made the
|
||||
// only diagnostic that names the failing resource print "invalid" every time.
|
||||
for (const f of FILES) {
|
||||
const src = code(f);
|
||||
if (!src.includes('load-error')) continue;
|
||||
assert.ok(!/\bdata\.url\b/.test(src), `${f}: a load-error names its resource in data.uri`);
|
||||
}
|
||||
});
|
||||
|
||||
test('parameters do not carry BOTH a type suffix and an As clause', () => {
|
||||
// `filePath$ As String` is a shape the reference never sanctions, and a parse error would stop
|
||||
// the script loading at all — the worst possible failure, since nothing would run to report it.
|
||||
for (const f of FILES) {
|
||||
for (const m of code(f).matchAll(/(?:Function|Sub)\s+\w+\s*\(([^)]*)\)/g)) {
|
||||
for (const param of m[1].split(',')) {
|
||||
assert.ok(!/[$%!#&]\s+As\s+/i.test(param),
|
||||
`${f}: parameter "${param.trim()}" has a type suffix and an As clause`);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('the storage root is resolved by probing, not assumed', () => {
|
||||
// Knowing only FLASH and SD meant that fitting real storage to a flash-booting player and moving
|
||||
// the deployment onto it resolved every derived path to a volume that was not there.
|
||||
const src = code('autorun.brs');
|
||||
const fn = src.slice(src.indexOf('Function StorageRoot'));
|
||||
for (const vol of ['SSD:', 'USB1:', 'FLASH:']) {
|
||||
assert.ok(fn.slice(0, 900).includes(vol), `StorageRoot() must consider ${vol}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('the widget storage path is absolute, on a real volume', () => {
|
||||
// "/cache" carries no drive specifier, so the widget's local storage has nowhere to persist.
|
||||
const src = code('autorun.brs');
|
||||
assert.ok(!/storage_path:\s*"\/[^"]*"/.test(src),
|
||||
'storage_path must name a volume — a bare "/path" is outside the writable volumes');
|
||||
assert.match(src, /storage_path:\s*StorageRoot\(\)/);
|
||||
});
|
||||
|
||||
test('no doubled quotes inside a string literal — BrightScript has no escape sequences', () => {
|
||||
// The one that cost a player its boot. `"{""width"":"` is not an escaped quote; it is three
|
||||
// adjacent string literals with no operator between them, and the compiler rejects the WHOLE
|
||||
// FILE: "ScriptLoadError: Syntax Error. (compile error &h02) in SSD:/autorun.brs(196)". The
|
||||
// display came up with nothing at all — not a broken feature, no player. A quote in a literal has
|
||||
// to come from Chr(34).
|
||||
for (const f of FILES) {
|
||||
const lines = fs.readFileSync(path.join(DIR, f), 'utf8').split('\n');
|
||||
lines.forEach((line, i) => {
|
||||
if (/^\s*'/.test(line)) return;
|
||||
// `""` hugged by non-delimiters is a literal trying to contain a quote; `, ""` or `("")` is
|
||||
// simply an empty string argument and is fine.
|
||||
assert.ok(!/[^\s(,=]""[^\s),]/.test(line),
|
||||
`${f}:${i + 1}: a string literal cannot contain a quote — build it with Chr(34)\n ${line.trim()}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test('every string literal on a line is closed', () => {
|
||||
// An odd number of quotes is the same class of failure: it takes the whole script down, and
|
||||
// nothing here can run it to find out.
|
||||
for (const f of FILES) {
|
||||
const lines = fs.readFileSync(path.join(DIR, f), 'utf8').split('\n');
|
||||
lines.forEach((line, i) => {
|
||||
if (/^\s*'/.test(line)) return;
|
||||
const beforeComment = line.split(/\s'(?=(?:[^"]*"[^"]*")*[^"]*$)/)[0];
|
||||
const quotes = (beforeComment.match(/"/g) || []).length;
|
||||
assert.equal(quotes % 2, 0, `${f}:${i + 1}: unbalanced quotes\n ${line.trim()}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
Loading…
Reference in a new issue