Failing to send any command after Command: [format]

[shinsuny]

Problem : After receiving format command response, next command from request Queue (i.e. ping command) is called but no response is generated so the request is in not-finished state hence no next command is requested.

The problem occurs on Raspberry Pi 2. No problem with Windows/Debian 64 bit.

※May be not related but on Raspberry Pi 2 Logger.debug also does not work, Logger.info is OK.

[benjaminkay93]

I’ll have a look at this later tonight, however I don’t have access to a hyperdeck right now so might be limited to what I can do :)

[shinsuny]

Thanks @benjaminkay93

I think its hard to reproduce and kind of unthinkable case.

「Multiple responses data are coming in one response.」 can you imagine by ↑ these words? :)

let me explain a bit more. Usually, in one response one response data comes. like following 「 200 ok」 or 「 202 slot info: slot id: 2 status: mounted volume name: DiskExFAT recording time: XXXX video format: XXXX device serial: XXXX device model: XXXX firmware revision: XXXX

」

but the problem is, in one response multiple response data is coming, the matter of the case is, first one is ASYNCHRONOUS and the second one is SYNCHRONOUS hence response is not handled for the second SYNCHRONOUS response since the response has come but not handled no further request is sent and system hangs.

Multiple response data looks like following: 「508 transport info: status: record video format: XXXX

204 device info: protocol version: 1.5 model: XXXX unique id: XXXX

」

Solution: some changes at response-handler.js → function onData(rawData) ` function onData(rawData){ if (rawData && rawData.includes('\r\n\r\n')) { const respArray = rawData.split('\r\n\r\n'); respArray.forEach(resp => { if (resp) { onDataOne(resp + '\r\n\r\n'); } }); } else { onDataOne(rawData); } }

function onDataOne(rawData){`

[tjenkinson]

Hmm interesting. We are testing the case where there is an asynchronous response before the synchronous one and it seems to be working 🤔 https://github.com/LA1TV/Hyperdeck-JS-Lib/blob/9bf46c2e72c2d851043339314a8f64b5d8291911/test/hyperdeck/hyperdeck-core.js#L103-L113

[shinsuny]

please note that two(or more) response data should be in one response, plus asynchronous should be at the top.

one response = 「 asynchronous response

synchronous response

」

it won't work because onData() method is not implemented to handle multiple response data in one response, I am sure about that.

[tjenkinson]

Ah I see. In which case I think it would be best to handle it here https://github.com/LA1TV/Hyperdeck-JS-Lib/blob/9bf46c2e72c2d851043339314a8f64b5d8291911/src/hyperdeck/response-handler.js#L29-L54

buffer += rawData; could be replaced with a loop that appends line by line, and the rest of the function would be in the loop

[tjenkinson]

if (!isBufferComplete()) { 
   return; 
} 

would become

if (!isBufferComplete()) { 
   continue; 
} 

[tjenkinson]

@sarveshwar-singh thanks for reporting! would be great if you could test #45 and see if it fixes it

[shinsuny]

oopse, just by looking at #45 i think it won't work because you are splitting by each line, since one response can be multiline it won't work.

i had given you the solution earlier.

1. Rename onData function to onDataOne function onDataOne(rawData) {

2. write response splitting logic in new onData function and call onDataOne from that. function onData(rawData){ if (rawData && rawData.includes('\r\n\r\n')) { const respArray = rawData.split('\r\n\r\n'); respArray.forEach(resp => { if (resp) { onDataOne(resp + '\r\n\r\n'); } }); } else { onDataOne(rawData); } }

and i have tested it, its working.

[shinsuny]

forget to mention that in multiresponse data, each response can be split by 2 line break (i.e. 2*\r\n) that's why i have used rawData.split('\r\n\r\n').

[tjenkinson]

hey @sarveshwar-singh isBufferComplete() will only return true though when there is 2*\r\n\r\n, or it is a single line response. I think with your code it might not work if there is a single line response between multiline ones.

e.g https://github.com/LA1TV/Hyperdeck-JS-Lib/pull/45/commits/5083ae532072e3934c75f7928ed87c1a579e2ec3

maybe I am missing something though? :)

[shinsuny]

let see in detail,

■one response data, one line response

"200 ok\r\n"

■one reponse data, multline response

"500 connection info:\r\nprotocol version: 1.5\r\nmodel: HyperDeck Studio Pro\r\n\r\n"

■multi response data, multline response(or can assume one line response in any combination)

"508 transport info:\r\nstatus: record\r\nvideo format: 1080i60\r\n\r\n204 device info:\r\nprotocol version: 1.5\r\nmodel: HyperDeck Studio Pro\r\nunique id: XXXX\r\n\r\n"

※cant show you image because cant do debugging on Raspberry Pi

lets see how new onData should work.

function onData(rawData) {

if (rawData && rawData.includes('\r\n\r\n')) {
   In case of 「one reponse data, multline response」and 「multi response data, multline response」
} else {
   Rest of other cases, that is the remainning「one response data, one line response」
}

}

I think with your code it might not work if there is a single line response between multiline ones.

I am using your old logic for single line response so it should work :) please let me know if you still not able to understand my logic.

45 will never work for 「multi response data, multline response」, in fact it will stop working for 「one reponse data, multline response」case.

[shinsuny]

not related to this problem but in the old onData function you should not return like this:

if (!isBufferComplete()) {
  return;
}

user will never come to know if unexpected format response came and your logic did not handle it.

you should

throw "Unknown response type.";

if you can't handle response.

[tjenkinson]

@sarveshwar-singh I think there is still a problem. First for your second point

The data on the socket could arrive in many small pieces, so we therefore can't throw an error because we are expecting the rest of the data to arrive later.

E.g. we might get

508 transport info:\r\nstatus: record\r\n

which is incomplete, because

• the first line has : at the end meaning we should expect multiple lines in the response
• it doesn't end with \r\n\r\n

and then later on get

video format: 1080i60\r\n\r\n

and now isBufferComplete() returns true because it got he \r\n\r\n at the end.

And for your first point. Here is an example that I think would break with your code (which is similar to what we are testing now in the unit test)

508 transport info:\r\nstatus: record\r\nvideo format: 1080i60\r\n\r\n104 disk full\r\n508 transport info:\r\nstatus: record\r\nvideo format: 1080i60\r\n\r\n

because it would call onDataOne() with

104 disk full\r\n508 transport info:\r\nstatus: record\r\nvideo format: 1080i60\r\n\r\n

the second time

You could try switching the code in the PR for yours, and running npm test, and see if it passes :)

[shinsuny]

The data on the socket could arrive in many small pieces, so we therefore can't throw an error because we are expecting the rest of the data to arrive later.

This clears the picture, so it means one response can come in multipart. That's why you are using a buffer.

I will test #45 :)

[shinsuny]

Hi @tjenkinson I tested #45

OK with first response,

Failing to handle second response,

buffer is not resetting properly.

[tjenkinson]

@sarveshwar-singh interesting thanks for testing. Could you provide all the values of rawData? I wonder if between the 2 commands there is actually \r\n\r\n\r\n 😱

[benjaminkay93]

Does the Hyperdeck not output a nice clear sign its finished responding to a request? is that what your thinking \r\n\r\n\r\n would be representative of?

[tjenkinson]

@benjaminkay93 according the pdf I'd expect a \r\n at the end of a single line response and a \r\n\r\n after a multiline response. But it looks like if there is a multiline followed by a single line response in the same socket data event it might be sending \r\n\r\n\r\n after the multiline response not \r\n\r\n

[shinsuny]

@tjenkinson i have not seen \r\n\r\n\r\n in reaponse.

First Response: "500 connection info:\r\nprotocol version: 1.5\r\nmodel: HyperDeck Studio Pro\r\n\r\n"

Second Response: "200 ok\r\n"

@tjenkinson do you have access to Hyperdeck device?

[tjenkinson]

@sarveshwar-singh unfortunately I don't have a hyperdeck. We have a test that has the format

"500 connection info:\r\nprotocol version: 1.5\r\nmodel: HyperDeck Studio Pro\r\n\r\n200 ok\r\n"

and it passes 🤔 https://github.com/LA1TV/Hyperdeck-JS-Lib/blob/5cb10b08cbf9fae89546b3ec78946c6f40ae3041/test/hyperdeck/response-handler.js#L53 https://github.com/LA1TV/Hyperdeck-JS-Lib/blob/5cb10b08cbf9fae89546b3ec78946c6f40ae3041/test/hyperdeck/response-handler.js#L99-L114

Anyway I have added a commit to ignore all empty lines before the response starts which will hopefully fix this. Would be great if you could check and then we can merge it

https://github.com/LA1TV/Hyperdeck-JS-Lib/pull/45/commits/5cb10b08cbf9fae89546b3ec78946c6f40ae3041

[shinsuny]

@tjenkinson yeah, the following will solve the problem that problem.

      if (buffer === '' && line.trim() === '') {
        // handle empty lines before the data
        // see https://github.com/LA1TV/Hyperdeck-JS-Lib/issues/44
        return;
      }

but I think we still need to figure out few problems.

1. format command has multiline response but the first line does not end with :

2. efficiency: because split is called at 2 places for one response, suppose if a response is 300 lines long then split will be called 300 + times of socket write for that response.

split 1 = 1 line long split 2 = 2 line long . . . split 298 = 298 line long ← inefficient split 299 = 299 line long ← inefficient
split 300 = 300 line long ← inefficient

which is practically inefficient because in my use case I need to manage disk having more than 600 files and when i read disk files then the response will be longer than 600 lines.

I am working on these problems, will it be good if I send you pull request later?

[shinsuny]

@tjenkinson

waiting for the reply.

https://forum.blackmagicdesign.com/viewtopic.php?f=12&t=67057&p=376465#p376465

[tjenkinson]

Great thanks! Yes happy to accept a pr :)

[tjenkinson]

I understand point 2 is a problem but I think point 1 is handled already though?

On 21 Nov 2017, at 10:15, SARVESHWAR SINGH notifications@github.com<mailto:notifications@github.com> wrote:

@tjenkinsonhttps://github.com/tjenkinson

waiting for the reply.

https://forum.blackmagicdesign.com/viewtopic.php?f=12&t=67057&p=376465#p376465

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/LA1TV/Hyperdeck-JS-Lib/issues/44#issuecomment-345963819, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADG-WdIA-xaJZF7yOKNM1ycMEKVdCa7-ks5s4pTFgaJpZM4QcvBx.

[shinsuny]

@tjenkinson

I think for solving problem 1 we needed re-write the logic of creating the buffer because it's response is multiline but it will pass single line response check.

I am thinking line response check logic as below;

Single Line: Response Starts with, 1XX_ or 200_ok

X = [ 0 - 9 ] _ = space

Multi-line: Apart from Single Line.

If Multi-line is in reading state then no need to check condition for Single Line/Multi-line.

[shinsuny]

@tjenkinson

I got format command problem on raspberry pi, it was working on Windows/Debian 64bit but don't you think old logic should not work in any case?

[tjenkinson]

Ah yes your right it we only get 'format\r\n' it would be treated as a single line response because there's no ':'. Could do with someone from black magic joining this issue

[tjenkinson]

@sarveshwar-singh I'm not sure if that will work because how will you distinguish whether

000 something\r\n

is a single line or multi line response.

some random data\r\n\r\n

might arrive later.

I think I have a workaround which is a bit of a hack but should hopefully make this reliable. If we get the first response above we could then send a ping command. Then after this we should either get the response to the ping immediately if the initial response was a single line, or we get more of the original response (followed by the ping response)

[tjenkinson]

Actually this is even worse than I was initially thinking. We can't handle the format case generically, where assume there might be data on the next lines if there is no :, because by doing this we could actually assume that an asynchronous response is just data for the first line.

E.g if we got

000 something \r\n
501 something async:\r\n
a: b\r\n\r\n

then we would end up treating this as a single multiline response that has data

501 something async:\r\n
a: b\r\n\r\n

instead of seeing this as a single line response followed by an asynchronous one.

We could check that the next line doesn't start with 3 digits and a space, but who says the hyperdeck format token couldn't be this structure?

I think the safest way is that we move to a whitelist of responses without : that have multiple lines, which I guess at the moment is just the format one

[tjenkinson]

This might be the best we can do without changes in the api

https://github.com/LA1TV/Hyperdeck-JS-Lib/pull/45/commits/ee61a27973a22b0cd649bb7930808fc04c9a970e

[shinsuny]

Ah yes your right it we only get 'format\r\n'

command=> format: prepare: exFAT response=>216 format ready\r\nxxxxxx\r\n\r\n

still no ":"

[shinsuny]

Let me explain how I decided on my logic.

1. Read full doc once more. http://documents.blackmagicdesign.com/HyperDeck/20170711-bdb5d3/HyperDeck_Manual.pdf

2. Goto page no 49, [ Response syntax ] , [ Successful response code] and [ Failure response codes ] 200 and 1XX are single line response. other 2XX responses like 204, 209, 214 are the multiline response. 5XX is always multiline.

3. ON that assumption is have analyzed each and every command response and its passing my logic.

4. Tested on every possible command and found working.

My logic will work on any bit pieces and order of combination of response as long as Hyperdeck does not change spec in future.

[shinsuny]

@tjenkinson

please have a look on the fix.

[shinsuny]

@tjenkinson

forget to mention, fix is overall 40 % faster as well.

[tjenkinson]

Ah you're right. So anything 2xx that's not 200 is always multiline! That means we don't need to use the colon to determine this.

Thanks for the pr will have a proper look later :)

One other thing we should add is a line buffer. At the moment we are assuming complete lines will arrive in the same data event but I'm not sure if this is always the case

[shinsuny]

@tjenkinson

That means we don't need to use the colon to determine this. Yes, at first I did not use ":" , buffer[0].search(/^1\d\d\s|200\sok$/) === 0 is enough to check for single line response , but && buffer[0].endsWith(':') === false add fail proof. single line response will never end with ":" , so its safer way.

http://documents.blackmagicdesign.com/HyperDeck/20170711-bdb5d3/HyperDeck_Manual.pdf

page 49 [ Basic syntax ]

The HyperDeck protocol is a line-oriented text protocol.

I have never seen any incomplete line response as yet, what about you?

[tjenkinson]

I haven't, but I think it's possible just like it's possible we only get one line at a time or several responses grouped together. It's the nature of sockets which are just a stream of bits.

[shinsuny]

Yeah, You are correct I have seen the opposite side, Hyperdeck waits for \n or \r\n to mark as command end.

on response arrival wait till data ending with \r\n, append it to buffer string like before.

[shinsuny]

sorry, i was busy with other projects. v1.3.0 looks nice, will try to test where I left :)