Release/1.3.5 rc.3 - #97
Open
lmtan91 wants to merge 7 commits into
Open
Conversation
The firmware implements the full v1.1.x settings byte; the Go port had only bit 0. Adds bit 1 (request), bit 2 (response), the message ID at Buffer[1], and UpdateBitPositionsForHeaderByte to skip it when decoding. Bit 1 is required to parse a §2.2 request payload: it carries POINT_IDs with no DATA_TYPE_ID, so without the flag a decoder cannot know whether to consume 6 data-type bits after each POINT_ID. No-op when both flags are clear, so existing uplinks decode identically.
Adds DecodeConfigRequest for the §2.2 request format. DecodeRubix cannot parse it: a request carries POINT_IDs with no DATA_TYPE_ID and no values.
Routes opt=3 from a device to handleConfigRequest, which previously fell through to 'unhandled LoRaRAW option: 3'. Resolves the desired push rate from UVP-1 (operator WriteValue first, then the device's last reported value), encodes a response body with the response flag and echoed message ID, and sends it directly via WriteToLoRaRaw. The write queue is deliberately bypassed: it sleeps timeOffAirDefault between sends and would miss the device's ~1s RX window. DequeueByIoNumber settles the pending write once the value has been delivered, fixing the point that otherwise stays write-pending forever. It returns nil when nothing is queued, which is what makes skipping the response cache safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k8XXowr8gprFWsqordsFs
Review finding on Task 4: DequeueByIoNumber removes an item mid-slice (matched by IoNumber), which breaks the invariant ProcessPointWriteQueue's DequeueWriteQueue relied on — that the front of the slice is still the item the worker took before it released the lock to do external work. If a config response settles a different queue entry while the worker is off encoding/ encrypting/transmitting/sleeping, the worker's blind pop-front then silently discards whatever item is now at the front instead. Adds PointWriteQueue.removePendingWrite(item), which removes a specific *PendingPointWrite by pointer identity under the queue lock, and switches all four removal sites in ProcessPointWriteQueue (getDevice error, getEncryptionKey error, encrypt error, retry-exhaustion) to use it with the exact item each call is holding. It removes what was actually processed, or nothing if that item is already gone — correct regardless of what else mutated the slice meanwhile. DequeueWriteQueue (blind pop-front) is kept but no longer called from the worker; grep confirms it has no other callers in the repo. Also adds TestWorkerRemovalSurvivesConcurrentDequeueByIoNumber, which reproduces the interleaving directly against the queue internals (queue two points, simulate the worker holding the front item, settle it via DequeueByIoNumber, then confirm the worker's own removal does not discard the second point). Confirmed failing against the pre-fix blind pop-front before switching it to removePendingWrite. Also adds TestResolveDesiredRateAcceptsBoundaries covering the previously-untested inclusive range boundaries (1 and 15000). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k8XXowr8gprFWsqordsFs
…need The config-sync work had picked up two things that belong to neither the request nor the response path. MDK_UO/UI/DO/DI (17-20) were added only so the codec test package would compile - decoder_test.go referenced them while serialmap.go did not define them, which is why that package had never built. Nothing in the config exchange uses those keys, and serialMap still has no metadata for them, so they bought a compiling test suite and nothing else. Removed along with TestSerialDataFull, the skipped test that was their only remaining caller. DequeueByIoNumber and clearPendingPushRateWrite were meant to settle the queued push-rate write once the device had taken the value. They cannot: the queue drops an item after WriteQueueMaxRetries * timeOffAirDefault (~25s by default) while the device syncs every 6h, so by the time a request arrives there is nothing left to dequeue and updateWrittenPointSuccess never runs. Removing them also removes the positional-invariant hazard they introduced, so writeQueue.go and writeQueueManager.go go back to their master versions, removePendingWrite included. Settling the write properly needs a queue entry that outlives its retries - a design change, not a patch - so it is left out rather than left broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k8XXowr8gprFWsqordsFs
Swept in by an over-broad 'git add -A' in the previous commit. They are local tool artifacts, not part of the feature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k8XXowr8gprFWsqordsFs
The LoRaRAW request/response exchange is a general mechanism - a device names
the points it wants, the gateway answers with their values - but every symbol
was named as though it existed only to carry configuration. Push rate is the
first thing a device asks for, not the only thing one ever could.
Split by what each part actually knows:
loraRawRequest.go the exchange itself: decode the request, resolve each
requested point, encode and send the response
configSync.go what a push rate is, what range it may take, and how to
read one off a device
Renames follow: DecodeConfigRequest -> DecodeRequestPayload,
handleConfigRequest -> handleInboundRequest, buildConfigResponsePayload ->
buildResponsePayload, and on the firmware side prepareConfigRequest ->
prepareRequest.
buildResponsePayload now takes points rather than a bare rate, which is what
lets it stop knowing about push rates at all. resolveRequestedPoint is the
single hook where per-point knowledge lives; new request-able points go there
and touch nothing else.
One behaviour change falls out of this: the handler now answers the points the
device actually asked for. It previously replied with the push rate whatever
the request named, which was harmless while UVP-1 was the only request anyone
sent, but wrong the moment that stops being true.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014k8XXowr8gprFWsqordsFs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.