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

Add Service Registration #2

Merged
merged 7 commits into from
Jan 31, 2014
Merged

Add Service Registration #2

merged 7 commits into from
Jan 31, 2014

Conversation

markjlorenz
Copy link
Contributor

Adds service registration with dnssd.ServiceRegister

  • Looks like dnssd_c.c was using spaces instead of tabs. I can put it back to spaces if that's better.
  • You'll want to change example/test.go#L5 to point to your repository.
  • I'm new-ish to Golang, and know almost nothing about C. I just copied your patterns. Feedback/criticism is appreciated.

@soh335
Copy link
Owner

soh335 commented Jan 30, 2014

Thank you for p-r. it seems to be good. But I found bug of my implementation.

here https://github.com/soh335/go-dnssd/blob/master/dnssd.go#L70 .

using only discover function, dont get a problem if deallocate context variable. However if calling register function case, should not deallocate it. Because deallocating it represents cancel registering to dns.

So I want to change api like this. https://github.com/soh335/go-dnssd/compare/feature;release_ctx?expand=1 . You require to call Release method to context expressly. And In this case, you can write register example like this.

func main() {
    ctx, err := RegisterService()
    if err != nil {
               panic(err)
    }
    defer ctx.Release()
    Discover()
}

func RegisterService() (*dnssd.Context, error) {
        txtRecords := map[string]string{
                "path": "/path-to-page.html",
        }

       rc := make(chan *dnssd.RegisterReply)
       ctx, err := dnssd.ServiceRegister(
               dnssd.DNSServiceFlagsSuppressUnusable,
               0,                // most applications will pass 0
                "My Server",
                "_http._tcp.",
               "",               // empty string ends up as local domain
               "",               // most applications do not specify a host
               3000,             // port the service is running on
                txtRecords,
                rc,
        )

       go dnssd.Process(ctx)

        if err != nil {
               return nil, err
        }

        registerReply, _ := <-rc
        fmt.Println("Register Reply: ", registerReply)
        return ctx, nil
 }

If you dont have problem, I plan to merge this branch to master . how about you ❓

@soh335
Copy link
Owner

soh335 commented Jan 30, 2014

Looks like dnssd_c.c was using spaces instead of tabs. I can put it back to spaces if that's better.

I prefer to use spaces for *.c file.

@markjlorenz
Copy link
Contributor Author

If you dont have problem, I plan to merge this branch to master . how about you ❓

Makes sense. OK by me.

I prefer to use spaces for *.c file.

OK, I will put it back to spaces tonight (EST)

Thanks for looking at my pull request!

@soh335
Copy link
Owner

soh335 commented Jan 31, 2014

thanks i merged to master my branch. I wait your commits ( using spaces for *.c file, example/test.go update )

@markjlorenz
Copy link
Contributor Author

Ok, everything should be ready to go:

markjlorenz@BabyGoat:go-dnssd(master|1m) → go run example/test.go
Register Reply:  &{My Server _http._tcp. 293693605.members.btmm.icloud.com.}
&{5 My Server _http._tcp. local.}
start resolve
&{5 My\032Server._http._tcp.local. BabyGoat.local. 3000 map[path:/path-to-page.html]}
&{5 My\032Server._http._tcp.local. 33 1 22 [0 0 0 0 184 11 8 65 97 98 121 71 111 97 116 5 108 111 99 97 108 0] 120}
&{0 0 47115 BabyGoat.local.}
&{5 BabyGoat.local. 10.0.1.5 120}
closed
markjlorenz@BabyGoat:go-dnssd(master|1m) →

soh335 added a commit that referenced this pull request Jan 31, 2014
@soh335 soh335 merged commit 1eb316e into soh335:master Jan 31, 2014
@soh335
Copy link
Owner

soh335 commented Jan 31, 2014

thanks !!

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