-
-
Notifications
You must be signed in to change notification settings - Fork 431
Restart discovery after re-initializing client. #1167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Otherwise, board discovery stops working after indexes update. Signed-off-by: Akos Kitta <[email protected]>
For reviewers: I am optimistic this PR should fix the discovery issue. At least, the nightly (f4a68e7) is broken here, the build from the PR works. Here is another way to verify. Connect at least one board to the machine before starting the IDE2. IDE2 starts, it's up and running, and shows the attached board(s). Wait for the indexes update, then detach the boards. IDE2 does not show the board changes from the nightly build. It works from this PR. |
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.
The code looks fine with me.
LGTM 🚀
UPDATE: fixed when using the build for e9cd290 |
I can also see a defect on my Windows machine.
Thank you! Do not spend too much time with it, the discovery also bogus on this slower Windows machine. |
Nothing else happens. IDE2 does not recognize the board. |
Signed-off-by: Akos Kitta <[email protected]>
Signed-off-by: Akos Kitta <[email protected]>
Signed-off-by: Akos Kitta <[email protected]>
Signed-off-by: Akos Kitta <[email protected]>
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.
I am able to reproduce the bug this PR addresses when using the latest build from the main
branch (7fed8fe).
Using the build from this PR, the discovery works extremely reliably. I was able to reproduce the issue reported at #1181 (comment), but very rarely even though I was specifically producing the conditions it occurs under.
So I would be satisfied with this fix even without the addition of the change proposed in #1181
Motivation
Before #1132 the core gRPC client was not re-initialized (
InitRequest
) after the indexes update. Hence; the CLI could not provide up-to-datecore search
results to the IDE2. Now, it is working in IDE2 but after the core gRPC re-initialization, the board discovery (BoardListWatchRequest
) is non-functional. This PR fixes it by sending out an event after the core client re-initialization so that the board discovery can restart in IDE2.From here:
How to test:
main
with f4a68e7.Change description
Other information
Reviewer checklist