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 missing '\r's and SSDP message formatting #2424

Closed
wants to merge 5 commits into from

Conversation

opticron
Copy link

This fixes missing carriage returns that were causing odd output when
debugging was enabled. This also adds the capability to define a custom
SSDP message formatter for devices that need more control over the
actual wire format of messages. For devices emulating Hue bridges, the
order of message elements is important as well as the ability to add
non-standard message elements.

This fixes missing carriage returns that were causing odd output when
debugging was enabled. This also adds the capability to define a custom
SSDP message formatter for devices that need more control over the
actual wire format of messages. For devices emulating Hue bridges, the
order of message elements is important as well as the ability to add
non-standard message elements.
@codecov-io
Copy link

codecov-io commented Aug 23, 2016

Current coverage is 27.80% (diff: 100%)

Merging #2424 into master will not change coverage

@@             master      #2424   diff @@
==========================================
  Files            20         20          
  Lines          3625       3625          
  Methods         335        335          
  Messages          0          0          
  Branches        656        656          
==========================================
  Hits           1008       1008          
  Misses         2441       2441          
  Partials        176        176          

Powered by Codecov. Last update 7b32e6a...4dc6b8d

@earlephilhower earlephilhower added merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Oct 1, 2019
@earlephilhower
Copy link
Collaborator

Thanks for your PR, but the core and libraries have changed enough that this PR now has a merge conflict.

Could you merge it manually with the latest core, so we can consider it for future releases?

@earlephilhower
Copy link
Collaborator

After looking at this I think it's not worth fixing with a new PR.

The \n to \r\n in debug messaged is the first big change, but nowhere else in the core do we do \r\n for debugging, so that's out as far as I'm concerned.

The other part involves adding an optional callback with 12(!) parameters before a SSDP message is formatted and no examples using this CB.

This might be worth a debug printf() (just dump the formatted message), but there's not been a clamoring for such a thing in the 3+ years this PR has been here.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants