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.

Sign up for GitHub

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

Jump to bottom

listen for announces on init and linkup #691

Merged
merged 1 commit into from
Sep 12, 2017
Merged

listen for announces on init and linkup #691

merged 1 commit into from
Sep 12, 2017

Conversation

ethan-stewart-iol
Copy link
Contributor

Currently, if starting two instances of daemon_cl, they do not connect to each other because neither listens for announce messages. This changes them from sending announces to listening. They then start announcing after an ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES event.

@andrew-elder
Copy link

I assume you mean two instances, each attached to a different port?

@ethan-stewart-iol
Copy link
Contributor Author

ethan-stewart-iol commented Sep 8, 2017 •
edited
Loading

Yes, that is correct. They are being started with sudo ./run_gptp eth1 and sudo ./run_gptp eth2.

@andrew-elder
Copy link

I'm not sure that I understand how doing startAnnounce() immediately is any different from waiting a constant timer delay time and then starting to send announce messages. Shouldn't it end up being the same?

@ethan-stewart-iol
Copy link
Contributor Author

The issue is that it never started listening for announce messages. It would be ok to start the ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES timer as well as the startAnnounce(), however when the ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES event is triggered, startAnnounce() is called (which would be redundant, but should work fine).

@andrew-elder
Copy link

Does anyone else have any comments on this pull request? I think is it ok.

pinealservo
pinealservo approved these changes Sep 12, 2017
View reviewed changes
Copy link
Contributor

@pinealservo pinealservo left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@andrew-elder andrew-elder merged commit 6cd1bff into Avnu:open-avb-next Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pinealservo pinealservo pinealservo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ethan-stewart-iol @andrew-elder @pinealservo