PantelisGeorgiadis / dcmjs-dimse

DICOM DIMSE implementation for Node.js using the dcmjs library
MIT License
68 stars 13 forks source link

RejectResult Constant is probably wrong #27

Closed caoshouse closed 2 years ago

caoshouse commented 2 years ago

Hi @PantelisGeorgiadis ,

Since I was coding a Typescript .d.ts file in order to use your wonderful library in my project, I noticed you defined the RejectResult constant in this way:

/**
 * Reject results.
 * @constant {Object}
 */
const RejectResult = {
  NotSpecified: 0,
  UnrecognizedPdu: 1,
  UnexpectedPdu: 2,
  UnrecognizedPduParameter: 4,
  UnexpectedPduParameter: 5,
  InvalidPduParameter: 6,
};

According to the DICOM specifications, Reject result should be "Transient" or "Permanent" (I did not actually look for corresponding hex codes).

This code:

this.sendAssociationReject( RejectResult.Transient, RejectSource.ServiceUser, RejectReason.CallingAeNotRecognized ); return;

will log:

2022-08-11T19:56:41.752Z -- INFO -- WEASIS -> Association reject [result: undefined; source: 1; reason: 3]

and this: console.log('\'' + Object.keys(RejectResult).join('\'|\n\r\'') + '\'');

will print:

'NotSpecified'| 'UnrecognizedPdu'| 'UnexpectedPdu'| 'UnrecognizedPduParameter'| 'UnexpectedPduParameter'| 'InvalidPduParameter'

Am I wrong?

Thanks

PantelisGeorgiadis commented 2 years ago

Thank you @caoshouse for your kind comments and for reporting this issue. You are absolutely right! Somehow the rejection and abortion constants got mixed! This is fixed now. I also took the opportunity to add some "human-readable" PDU logging functions.

Once you are ready with your .d.ts file and, of course you wish to share, we would love a related PR!

caoshouse commented 2 years ago

Thank you Of course I will

PantelisGeorgiadis commented 2 years ago

Thank you once again @caoshouse for finding and reporting this. I'm closing the ticket now and looking forward for your typescript type information PR!