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

Implement required connectionManagerService and mediaReceiverRegistrarService #86

Merged
merged 48 commits into from
Jan 20, 2022

Conversation

tsynik
Copy link
Contributor

@tsynik tsynik commented Sep 8, 2021

this fix DLNA browse on LG TVs and under Windows 10 Explorer

PR also have some addons from rclone repo for Samsung TVs (some of them still have browse issues)

Probably fix for #40 #78 #73

@anacrolix
Copy link
Owner

The formatting and imports are messed up. Try running goimports -w .. That leaves these failures:

$ go test ./...
# github.com/anacrolix/dms/dlna/dms [github.com/anacrolix/dms/dlna/dms.test]
dlna/dms/mrrs_test.go:21:38: undefined: testURL
dlna/dms/mrrs_test.go:21:69: undefined: mustMarshalXML
?   	github.com/anacrolix/dms	[no test files]
--- FAIL: TestContentFeaturesString (0.00s)
    dlna_test.go:14: DLNA.ORG_OP=10;DLNA.ORG_CI=1;DLNA.ORG_FLAGS=01700000000000000000000000000000
FAIL
FAIL	github.com/anacrolix/dms/dlna	0.117s
FAIL	github.com/anacrolix/dms/dlna/dms [build failed]
ok  	github.com/anacrolix/dms/misc	(cached)
?   	github.com/anacrolix/dms/play/termsig	[no test files]
?   	github.com/anacrolix/dms/rrcache	[no test files]
?   	github.com/anacrolix/dms/soap	[no test files]
?   	github.com/anacrolix/dms/ssdp	[no test files]
?   	github.com/anacrolix/dms/transcode	[no test files]
ok  	github.com/anacrolix/dms/upnp	(cached)
?   	github.com/anacrolix/dms/upnpav	[no test files]
FAIL

@tsynik
Copy link
Contributor Author

tsynik commented Sep 9, 2021

The formatting and imports are messed up. Try running goimports -w ..

This part was from rclone's https://github.com/rclone/rclone/blob/master/cmd/serve/dlna/dlna_test.go for new service, not checked it. sorry. it.s ok now:
ProBooK:dms nikk$ go test ./...
? github.com/anacrolix/dms [no test files]
ok github.com/anacrolix/dms/dlna 0.011s
ok github.com/anacrolix/dms/dlna/dms 0.030s
ok github.com/anacrolix/dms/misc 0.011s
? github.com/anacrolix/dms/play/termsig [no test files]
? github.com/anacrolix/dms/rrcache [no test files]
? github.com/anacrolix/dms/soap [no test files]
? github.com/anacrolix/dms/ssdp [no test files]
? github.com/anacrolix/dms/transcode [no test files]
ok github.com/anacrolix/dms/upnp 0.015s
? github.com/anacrolix/dms/upnpav [no test files]

@tsynik
Copy link
Contributor Author

tsynik commented Sep 9, 2021

The formatting and imports are messed up

@anacrolix do I need to apply gofmt from go1.17 also?
gofmt -l -w .
dlna/dms/dms_others.go
dlna/dms/dms_unix.go
dlna/dms/dms_unix_test.go
dlna/dms/dms_windows.go
play/attrs.go
play/bool.go
play/closure.go
play/execbug.go
play/execgood.go
play/ffprobe.go
play/mime.go
play/parse_http_version.go
play/print-ifs.go
play/scpd.go
play/soap.go
play/transcode.go
play/url.go

generates this:
//go:build ignore
// +build ignore
etc

@anacrolix
Copy link
Owner

Yes please.

dlna/dms/cms-desc.go Outdated Show resolved Hide resolved
dlna/dms/dms_others.go Outdated Show resolved Hide resolved
dlna/dms/cds-desc.go Outdated Show resolved Hide resolved
@anacrolix
Copy link
Owner

Do we need any license notice from rclone?

@tsynik
Copy link
Contributor Author

tsynik commented Sep 9, 2021

Do we need any license notice from rclone?

here we don't have direct code from rclone - just similar logic.

@rsteube
Copy link
Contributor

rsteube commented Oct 11, 2021

Oh awesome, yes this fixes #78. Just tried it out and works great so far. Thanks for the work.

@anacrolix
Copy link
Owner

I mentioned on the probable fix issues to try this PR. Thanks for your great work @tsynik .

@anacrolix
Copy link
Owner

I'm considering squashing and merging this. If I can get some more feedback from users that it's working for them. Thanks again to @tsynik for the good work.

@anacrolix
Copy link
Owner

Okay let's give it a go. I'm not hearing any more feedback and you put a lot of effort in to this.

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.

4 participants