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

examples: connect to all peers in example mdns chat app #1798

Merged
merged 2 commits into from
Oct 9, 2022

Conversation

karthik340
Copy link
Contributor

Fixes #1797

Added if condition to make sure it is not attempting self dial .

Added for loop so that multiple peers can join chat .

Copy link
Collaborator

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

As of #1661, we shouldn't be discovering ourselves. Is this check really needed?

@marten-seemann marten-seemann changed the title Fixed bug in example mdns chat app. examples: fixed bug in example mdns chat app Oct 6, 2022
@karthik340 karthik340 closed this Oct 7, 2022
@karthik340
Copy link
Contributor Author

karthik340 commented Oct 7, 2022

In #1661 this issue is fixed and go.mod in examples is upgraded recently with the new version of libp2p, so I don't think these changes are required.
but new peers cannot connect to more than one peer, for loop is required but it is not the purpose of the example.

@karthik340 karthik340 reopened this Oct 7, 2022
fmt.Println("Connected to:", peer)
for { // allows multiple peers to join
peer := <-peerChan // will block untill we discover a peer
if peer.ID != host.ID() { // cannot connect to itself
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not needed, as of #1661.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this logic

Copy link
Collaborator

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

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

You should rebase/merge master. You’re changes would regress the examples :)

"github.com/libp2p/go-libp2p/core/protocol"
"github.com/libp2p/go-libp2p-core/crypto"
"github.com/libp2p/go-libp2p-core/network"
"github.com/libp2p/go-libp2p-core/protocol"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You want to undo this. This is referencing the old interfaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did rebase

@marten-seemann marten-seemann dismissed MarcoPolo’s stale review October 9, 2022 14:44

rebase was executed correctly

@marten-seemann marten-seemann changed the title examples: fixed bug in example mdns chat app examples: connect to all peers in example mdns chat app Oct 9, 2022
@marten-seemann marten-seemann merged commit 63b8803 into libp2p:master Oct 9, 2022
julian88110 added a commit that referenced this pull request Oct 11, 2022
examples: use circuitv2 in relay example (#1795)

* Updated relay example to use circuit-v2

* fixed incorrect import

* updated test fixture

* upgrade dependencies

* merged upstream

* Implemented suggested changes in upstream PR

* Implemented suggested changes in upstream PR

roadmap: fix header level on "Mid Q4" (#1818)

examples: connect to all peers in example mdns chat app (#1798)

* Fixed bug in example mdns chat app.

* Added continue, so that if connection to other host fails, will go back to discovering.
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.

Bug in example mdns chat
3 participants