Skip to content
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

Candidate changes to provide a way for user code to tell if Senders are really ready to send data #66

Merged
merged 2 commits into from
Oct 3, 2023

Conversation

bieryAtFnal
Copy link
Contributor

In recent running of automated integration tests, I noticed occasional complaints about timeouts sending HSIEvents from FakeHSIEventGenerators to TimingTriggerCandidateMakers.

I found that the underlying problem was that the HSIEvent NetworkSender instance was not quite fully ready to send messages.

In order to provide a way for the FakeHSIEventGenerator code to check whether the Sender instance is really ready, I've prototyped a new method that provides this information. These changes are the subject of this PR.

As an aside, I want to mention that I found the existing contents of the NetworkSenderModel constructor very confusing. It had a try/catch block surrounding a method call that doesn't throw an exception. To help make things more clear, I removed that try/catch block. With this change, I now correctly see the "Initial connection attempt failed" log message. Previously, I was not correctly seeing this message in the logs.

The changes in this PR are precursors to ones in a companion PR that I will file soon in the hsilibs repo.

Copy link
Member

@eflumerf eflumerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, builds and integration tests run

@bieryAtFnal bieryAtFnal merged commit 56c3456 into develop Oct 3, 2023
3 checks passed
@bieryAtFnal bieryAtFnal deleted the kbiery/sender_ready_method branch October 3, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants