API: add warp/get; accept $ in assemble, hex byte patterns in search, both register shapes in vic/write - #116
Conversation
|
warp/get approved cpu/search rejected hex byte patterns approved vic/read and vic/write disagreed on shape needs more details. |
…earch, both register shapes in vic/write
Four API round-trip gaps that make automation awkward:
- warp could be set but not read, so a client could not verify its own action.
Added CDebuggerApi::GetWarpSpeed() over the existing GetSettingIsWarpSpeed()
and a %s/warp/get endpoint; %s/warp/set now returns the resulting state.
- cpu/assemble rejected "lda #$07" with 'Not a number after #'. '$' is the
canonical 6502 notation, so this is the first thing a client tries. Strip it
at the caller, exactly as CViewMonitorConsole already does, rather than
teaching the shared assembler grammar about '$' -- that would turn the
malformed "lda #$" from a clean 400 into a silent A9 FF write.
- cpu/search only accepted mnemonics, so "a9 ??" came back as
unknown_mnemonic. A first token of exactly two hex digits is now read as a raw
opcode byte; this cannot collide with a mnemonic, since every name in the
opcode table is three characters long.
- vic/read hands back [[reg, val], ...] while vic/write demanded {reg: val},
so feeding a read result straight back failed with type_error.302.
vic/write now accepts both shapes and rejects anything else with 406 instead
of throwing out of the handler.
Verified on Debian 13: warp/get tracks warp/set, "lda #$07" assembles to a9 07
while "lda #$" still fails, "a9 ??" returns 55 matches that all start with A9,
and a vic/read result can be written back unchanged.
Implements the shape agreed in the PR discussion. The old state was a trap:
vic/read emitted [[reg,val],...] pairs in RANDOM order (an accident of
serializing std::unordered_map through nlohmann), while vic/write demanded an
object -- so a read result could not be written back, and neither shape could
carry what chip registers actually need.
Canon: ordered list of records.
read: {"registers": [17, "$D016", "0x19"]} ->
{"registers": [{"reg":17,"addr":53265,"value":155}, ...]}
in REQUEST order (duplicates in the request are legal)
write: {"registers": [{"reg":17,"value":155}, {"addr":"$D019","value":255},
{"reg":17,"value":27}]}
executed in order, duplicates included
Why a list of records and not either trap branch: a JSON object carries
neither ORDER nor DUPLICATES, and both are semantics on memory-mapped chips
(interrupt acks, $D011/$D012 raster sequences, gate-off/gate-on in one batch);
pair lists are positional and undocumentable. Records are self-describing and
extensible. Round-trip holds: a read response is a valid write request.
Applied consistently to vic, cia, sid and drive1541/via. Reads emit records in
request order; cia/via records carry num+addr so a read written back lands on
the same chip; sid keeps its burst semantics (the batch describes final state,
so a later duplicate wins -- documented in the endpoint description).
Backward compatible: writes still accept the legacy object and the legacy pair
list; anything else is 406 instead of an exception-turned-500.
Also fixed on the way:
- sid register indices are now bounds-checked; previously a register number
>= C64_NUM_SID_REGISTERS wrote straight past the sidRegs array
- Atari antic/gtia/pokey/pia writes parsed register keys with base-10 stoi(),
so "0x18" silently became register 0 -- now the same dec/hex parser the C64
endpoints use
Verified behaviorally (gate script, same binary): records in request order
with duplicates, all three write shapes land (read-back witness, masked for
VIC's unconnected always-1 bits), read->write round-trip, junk shape -> 406,
$DD02 resolves to CIA2, sid burst accepts records. All red on the previous
binary, 9/9 green after.
dddb74e to
8a14318
Compare
|
Reworked as discussed -- the shim is gone, the canon landed. Commit The contractread -- request unchanged; response is an ordered list of records, in request order (the old // c64/vic/read {"registers": [17, "$D016", "0x19"]}
{"registers": [
{"reg": 17, "addr": 53265, "value": 155},
{"reg": 22, "addr": 53270, "value": 200},
{"reg": 25, "addr": 53273, "value": 4}
]}write -- ordered list of Applied consistently to
Fixed on the way
VerifiedBehaviorally, on the built binary, with a gate script whose every test is red on the previous binary (0/9) and green after (9/9): request-order with duplicates, all three write shapes (read-back witness, masked for VIC's unconnected always-1 bits), read->write round-trip, junk shape -> 406, CI on my fork:
So the merge order stays: #117, then #118/#119, then this. (One footnote for honesty: the combined Windows run needed a rerun -- the first attempt died with lld-link crashing on the runner, exit 0xC0000005, zero compile errors in the log. Tool crash, not code; the rerun went green with no changes.) |
My mistake when splitting the original work into separate PRs: the warp/get endpoint hunk in CDebuggerServerApi.cpp went into #114 (the commits were split by file, and that file carried changes belonging to two topics), while the CDebuggerApi::GetWarpSpeed() it calls stayed in the still-open #116. With #114 and #115 merged and #116 not, master does not compile on Linux: CDebuggerServerApi.cpp:112: error: 'class CDebuggerApi' has no member named 'GetWarpSpeed'; did you mean 'SetWarpSpeed'? This adds just that method (declaration + trivial forward to the existing CDebugInterface::GetSettingIsWarpSpeed), nothing else. Verified: master + this patch builds clean on Debian 13 / GCC 14. Note: the macOS and Windows CI failures on the same runs are unrelated to the merged PRs -- macOS dies in MTEngineSDL's Libtool step (Xcode 26.6 runner image), Windows in bundled SDL's SDL_endian.h (_m_prefetch builtin clash). Details in a comment on the run.
Four small API gaps that make the WebSockets API awkward to drive from a script. All are
additive or backward compatible.
warp/getwarp/setexists, but there is no way to read the state back, so an automated client cannotverify its own action. The lower layer already has a real getter --
CDebugInterfaceVice::GetSettingIsWarpSpeed()reads VICE'swarp_mode_enabled-- it was justnever exposed. Added
CDebuggerApi::GetWarpSpeed(), a%s/warp/getendpoint, and made%s/warp/setreturn the resulting state so one round trip is enough.cpu/assemblerejected$lda #$07returned400 {"error":"assemble_error","message":"Not a number after #"}, whilelda #7worked.$is the canonical 6502 notation, so it is the first thing a client tries.The
$is stripped at the caller inCDebuggerApi.cpp, exactly the wayCViewMonitorConsole.cpp:2753-2766already does it ("Remove '$' characters (assembler ishex-only)"). I deliberately did not teach
CViewDisassembly::AssembleGetTokenabout$:that turns a malformed
lda #$from today's clean 400 intoGetHexNumber() == -1->instructionValue = 0xFFFF-> HTTP 200 and a silentA9 FFwrite to memory. The caller-sidestrip keeps that input failing.
There was also a comment at the patched spot claiming the assembler "handles '$' by stripping
it internally via token parsing" -- it does not; it is replaced by the code that actually does.
cpu/searchrejected hex byte patternsa9 ??returned400 {"error":"unknown_mnemonic","mnemonic":"A9"}; only mnemonics such aslda ??were accepted. Hex bytes are the most common way to write a pattern by hand.A first token of exactly two hex digits is now read as a raw opcode byte. This cannot collide
with a mnemonic: every name in the opcode table is three characters long, so the hex-looking
ADC,BCCandDECare never two characters.vic/readandvic/writedisagreed on shapevic/readreturns a list of pairs[[27,0],...](the handler builds anunordered_map<u64,u8>, and nlohmann serializes a map with non-string keys as an array ofpairs), while
vic/writerequired an object{reg: value}. Feeding a read result straightback therefore failed with
500 [json.exception.type_error.302] type must be number, but is array-- and read-modify-write is the natural cycle for VIC registers.vic/writenow accepts both shapes. Anything that is neither array nor object is answeredwith 406 instead of throwing out of the handler into a 500.
vic/readis unchanged, soexisting clients are unaffected.
Verification
Debian 13, same binary before/after:
warp/getafterwarp/set true/falsetrue/falseassemble "lda #$07"a9 07assemble "lda #$"search "a9 ??"vic/writefed avic/readresult