senaranya/HL7

Segment separators are not correct

arnowelzel opened this issue · 14 comments

In certain places the segment separators are defined as following:

$this->hl7Globals['SEGMENT_SEPARATOR'] = '\n';

However - this is not correct:

  1. The sequence '\n' will translate to two characters and not to a newline.
  2. The separator must be "\015" (ASCII 13, CR) and not just a newline.

You can override the segment separator while creating Message object:

$msg = new Message("MSH|^~\\&|1|\rPV1|1|O|^AAAA1^^^BB|", ['SEGMENT_SEPARATOR' => '\r']);

Also see here: https://healthstandards.com/blog/2007/09/24/hl7-separator-characters/

The segment separator is not negotiable. It is always a carriage return (ASCII 13 or HEX 0D).

And for my particular use case I do not create a new message. The fragment is as follows:

// $message is a HL7 message recieved from some other system
$request = $hl7->createMessage($message);

// Create a response to be sent back
$response = new ACK($request);

Please use new Message() instead of new HL7() that you might have used. The documentation at https://github.com/senaranya/HL7#creating-new-messages describes various options of creating the Message object from a HL7 string.

I know \n is non-standard, but a lot orgs have configured their RIS to accept it. Making a core change in the default such as this is going to break a lot of systems that are using this library.

I understand the problem. But how to fix the behaviour for new ACK()? This will create a message with the wrong separator which is even literally \n (two characters: backslash and n) and not a newline. At least you should replace '\n' by PHP_EOL or "\n".

You can pass a Message object to the constructor of ACK:

$msg = new Message(<HL7 string>, ['SEGMENT_SEPARATOR' => <separator character>]);
$response = new ACK($msg);

Did the above work for you?

No, the ACK still contains "\n" (backslash followed by "n" (ASCII 92, ASCII 100)) and not CR (ASCII 13) as segment separator.

Edit: the code fragment I tried:

$request = new Message($message, ['SEGMENT_SEPARATOR' => "\015"]);
$response = new ACK($request);
$responseData = $response->toString();

I think the problem is, that the constructor of ACK first creates a message but without any optional parameters for the segment separators and just builds a new message with the default values.

Maybe it would be better to just have one instance of HL7 which is then used for all operations and holds a set of defaults for messages which can be passed in the constructor would be even better, so one does not always have to pass the globals as a parameter.

So this could be used then like this:

$hl7 = new HL7(['SEGMENT_SEPARATOR' => "\015"]);

$request = $hl7->createMessage($message);
$response = $hl7->createACK($request);
$responseData = $response->toString();

I just updated my pull request with the proposed changes.

Thanks for the pull request. We shouldn't use the HL7 class though, it is not tested and not used anywhere. I've kept it thinking someday it'd be a front facade for creating HL7 messages, but Message class is working pretty well for everyone so far. So I might remove HL7 class for good.

So please replace new HL7() with new Message() wherever you've used it in the code.

->toString() takes a boolean argument. If you call ->toString(true), it's going to expand '\n' or '\r' to the corresponding characters.

Thank you for the pull request, appreciate your contribution. We'll need to make a few changes though:

  • Revert any changes in HL7.php (as explained above)
  • Add unit tests in AckTest.php covering the updates
  • Create a section under ###ACK in readme.md describing how to create an ACK response.

I'm sorry... but ->toString(true) also doesn't work. This will just replace the two characters \n (literally a backslash followed by n and not a newline) to a single character LF (ASCII 10).

Also see here:

            $message .= $pretty
                ? str_replace(['\r', '\n'], ["\r", "\n"], $this->segmentSeparator)
                : $this->segmentSeparator;

Please note: inside single quotes PHP does not expand \n as newline but literally as two separate characters! Only \' can be used to escape a single quote and \\ to escape a backslash character. Also see here:

https://www.php.net/manual/en/language.types.string.php#language.types.string.syntax.single

Maybe it would be a good idea, to expand the constructor of ACK so it can accept an optional $hl7Globals parameter to be passed to the message which it creates. Also see my updated pull request.

->toString(true) is used by Connection class before sending the HL7 message, so it expands literal \r and \n to CR and NL characters respectively.

Thanks for the update in pull request. I'll merge it once tests and readme are added. If you'd want me to do it, I can do over the weekend.

Thank you. It's not that urgent - just take your time. For now I have a local workaround which will do until there is an official update of your component. Thanks for your work!