This repository has been archived by the owner on Mar 26, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 12
Add a register method and use it when connecting #26
Open
valpackett
wants to merge
2
commits into
jonas-schievink:master
Choose a base branch
from
valpackett:register
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the device runs out of connections? Is that something that can happen? Will it reject new connections from being registered or will older ones be dropped automatically?
(this is something that could easily happen when an app either calls
process::exit
or is killed via Ctrl+C)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my old-clone-firmware-whatever device, it never runs out of connections. It always has four, and newer ones just overwrite older ones, it's like a 4-size ring buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, good. I was going to test this on my (original) J-Link but noticed that the
list
example is failing with<USB error while reading from device: Overflow>
, so it looks like this doesn't work anymore with some hardware? (this is a J-Link Base Compact version 10.1)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm. Where is it failing? Is that after receiving the reply for registration? If so, can you dump the lengths (after
// Receive connection table
) and/or the whole receivedbuf
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just the read of 76 Bytes failing:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh, so it is an actual USB level error? hmm. Does OpenOCD work with this device?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should work. I believe we need to change how reads/writes work, because libjaylink seems to use an extra buffer, while we directly call into libusb (which will refuse to read more than 76 Bytes into the 76 Byte buffer). You might not be hitting this problem because the clone firmware always returns a fixed-length response here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the FW I have always returns 76. Hmm. I guess we could use an arbitrary big buffer just for this? ¯\_(ツ)_/¯