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

Handle C-ECHO request in storeSCP CLI tool #338

Merged
merged 4 commits into from Apr 11, 2023

Conversation

PierreBou91
Copy link
Contributor

This commit adds support for handling C-ECHO requests in the storeSCP CLI tool. With this update, the storeSCP tool can now process incoming C-ECHO requests and send appropriate responses.

Related to issue #337

Before screenshot:
before

After screenshots:
after

It appears that the main functionality of the storescp is maintained:
send

All test passed after cargo test

The changes in transfer.rs were relevant to:

  1. Remove duplicates SOPClasses.
  2. Add the Verification SOP Class necessary for the presentation context.

Please tell me if any additions / modification / deletion are needed.

This commit adds support for handling C-ECHO requests in the storeSCP CLI tool. With this update, the storeSCP tool can now process incoming C-ECHO requests and send appropriate responses.
@Enet4 Enet4 added enhancement A-tool Area: tooling C-storescp Crate: dicom-storescp labels Apr 4, 2023
@Enet4 Enet4 self-requested a review April 4, 2023 14:18
Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this feature, @PierreBou91!

Could you please rustfmt this source file? The changes added do not seem to follow the rest of the formatting.

Minor suggestion inline.

storescp/src/main.rs Outdated Show resolved Hide resolved
@PierreBou91
Copy link
Contributor Author

PierreBou91 commented Apr 6, 2023

I forgot to fmt, apologies for that.

Thanks for the suggestion regarding the switch to InMemDicomObject::from_element_iter, I also made the modification for create_cstore_response as I felt like it was the same scenario.

storescp/src/main.rs Outdated Show resolved Hide resolved
PierreBou91 and others added 2 commits April 6, 2023 13:09
Co-authored-by: Eduardo Pinho <enet4mikeenet@gmail.com>
@PierreBou91
Copy link
Contributor Author

I also got rid of the useless array in create_cstore_response so it matches your modifications. The built version seemed to work fine.
Thanks for your time and advices @Enet4 !

Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried it out in practice and it appears to be responding well to C-ECHO requests. Found no regressions of the standard storage service either, so this is good to enter! Thank you very much!

@Enet4 Enet4 merged commit e60cbbf into Enet4:master Apr 11, 2023
4 checks passed
@PierreBou91 PierreBou91 deleted the feature/handle-cecho-storescp branch April 16, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tool Area: tooling C-storescp Crate: dicom-storescp enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants