fix: status buffer split and missing binding-site lines - #3027
Conversation
Drain the full firmware status buffer instead of stopping after 20 USB transfers, avoid re-reading on every connection-state flicker, and keep binding-site lines in the error panel (#3025). Co-authored-by: Cursor <cursoragent@cursor.com>
|
@kareltucek This fix works for me. How about you? |
|
Yes, seems to work fine. |
| // Only pull the status buffer when the device newly becomes available. | ||
| // Re-reading on every connection-state field change drains leftovers from | ||
| // a previous partial read (or an empty buffer) and overwrites the UI (#3025). | ||
| if (becameAvailable) { |
There was a problem hiding this comment.
The getDeviceConnectionStateAsync is definitely overloaded. It worth to split and refactor but it bigger task.
But I am not sure we have to read the status buffer only connecting the right half to the computer.
@kareltucek Please validate my knowledge?
As far as I know the firmware could generate status buffer content when left half oscillating or modules are connecting or disconnecting to each half.
Maybe worth to extend the response of GetDeviceState (0x09) with a status buffer has content flag like isMacroStatusDirty. Or use the isMacroStatusDirty to indicate the status buffer has value.
If in introduce a status buffer flag indicator then we could delete the direct status buffer checks after the user configuration has been saved.
@kareltucek What do you think about it
There was a problem hiding this comment.
Maybe worth to extend the response of GetDeviceState (0x09) with a status buffer has content flag like isMacroStatusDirty.
I believe the isMacroStatusDirty (GetDeviceState (0x09) byte 7) does exactly that.
I.e., changes to 1 whenever Agent has started reading the buffer but has not finished, and whenever a new content is added into the buffer.
But I am not sure we have to read the status buffer only connecting the right half to the computer.
We definitely have to read it whenever isMacroStatusDirty becomes true.
There was a problem hiding this comment.
(I.e., this code is probably wrong and introduces a bug.)
There was a problem hiding this comment.
Later in this function the
line handle theisMacroStatusDirty.
So this condition does not introduce bug. But based on what you wrote this condition does not need. Enough to check the isMacroStatusDirty byte.
thx
There was a problem hiding this comment.
I believe the isMacroStatusDirty (GetDeviceState (0x09) byte 7) does exactly that.
I deleted this code and I run the diagnose usb macro and the isMacroStatusDirty was false so we need to check the status buffer every time when the connection changing.
I reverted the becameAvailable the iteration read was that fixed the original bug.
I left the name of isMacroStatusDirty as is. because looks like it set the value only when macro invalid.
@kareltucek If you will have time please test again. I tested but double check is better :)
|
|
||
| const url = `#/macro/${macro.id}?actionIndex=${macroActionIndex}&lineNr=${lineNr}&columnNr=${columnNr}&inlineEdit=true`; | ||
| const newLine2 = `${escapeHtml(line1Result[1])}<a href="${url}">${escapeHtml(line1Result[2])}</a>`; | ||
| // Keep binding-site lines and nested location-stack lines (firmware may emit more than 3). |
There was a problem hiding this comment.
As a future plan would be nice to write some test for these usecases
3 participants
Summary
Fixes #3025
Test plan
diagnose usbon UHK 60 with a long status buffer and confirm Agent shows the full message (not a short leftover fragment)Made with Cursor