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

Allow changing ImplementationClassUID and ImplementationVersionName by providing the appropriate global variables. #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

doskachok
Copy link

This change make it possible for client's code to configure the values of ImplementationClassUID and ImplementationVersionName. By default OFFIS values will be used.

It is needed to identify and distinguish implementation environments from each others, especially in case when the module is used by several product types.

…y providing the appropriate global variables.

By default OFFIS values will be used. This change is needed to be able to use different implementation identification information for different poducts types.
@jriesmeier
Copy link
Member

Thank you for your contribution. I personally don't like the introduction of two new OFGlobal instances for this purpose. Why don't you extend the underlying ASC_createAssociationParameters() function accordingly, i.e. by two optional parameters for the Implementation Class UID and the Implementation Version Name? This seems to be more natural to me as it is the function where the two values are copied to the T_ASC_Parameters structure.

@malaterre
Copy link
Contributor

I personally don't like the introduction of two new OFGlobal instances for this purpose.

+1 You never understand why a call in a random function from your process affect some other random function from the same running process...

@doskachok
Copy link
Author

Thanks for the review! I will have a look how to do it with extending ASC_createAssociationParameters. At this moment I can say that it will require to add setters for these values at least in DcmSCU and DcmSCP. They are also used when writing files, not only in association, thus I will probably need to add setters there as well.

@jriesmeier
Copy link
Member

jriesmeier commented Sep 7, 2022

Right, the information is also used in the file meta information header (of the DICOM file format). Alternatively to setting the values at runtime, there could be a define that declares two extern const char * global variables/constants that have to be defined in the user code. This would make sure that the user program always uses the same identifying information, but it would still allow for specifying different information for different user programs based on the same DCMTK library.

@doskachok
Copy link
Author

I just created draft PR that implements "optional parameters" suggestion, please have a look: #69

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.

3 participants