Skip to content

Revise dicom-app-common library - #794

Merged
Enet4 merged 3 commits into
masterfrom
change/app-common/revise
Jun 25, 2026
Merged

Enet4 merged 3 commits into
masterfrom
change/app-common/revise

Conversation

@Enet4

@Enet4 Enet4 commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Just a few clean-ups and bug fixes here and there.

  • Bring support for PeerCert::Ignore when setting up a TLS client, via a server certificate verifier that skips verification.

    • A warning is always raised in clients when this option is set, because one should rather add trusted certificates on this end.
  • Rename TLSProtocolVersion to TlsProtocolVersion

  • Fix system roots flag option, which had ArgAction::SetFalse when it should have been enabled on --system-roots

  • Rename a few error type variants and split others

    • remove anti-pattern of Io and Rustls error wrappers, replace them with more informative variants
  • make error type non-exhaustive

  • enabled use of value delimiter in suitable args, so that comma-separated values would actually work

  • Adjust documentation

  • [storescp] [storescu] adjust log filters: add dicom_app_common=debug in verbose mode

@Enet4 Enet4 added A-tool Area: tooling C-storescu Crate: dicom-storescu C-storescp Crate: dicom-storescp labels Jun 20, 2026
Enet4 added 2 commits June 20, 2026 12:21
- add support for PeerCert::Ignore in client_config,
  using a server certificate verifier that skips verification
   with a warning
- Rename `TLSProtocolVersion` to `TlsProtocolVersion`
   - and rename variants to match `ProtocolVersion` from rustls
- fix system roots flag option,
  which had `ArgAction::SetFalse` when it should have been enabled
- rename a few error variants and split others
   - remove anti-pattern of Io and Rustls error wrappers,
     replace with informative variants
- make error type non-exhaustive
- enabled use of value delimiter in suitable args
- add and adjust documentation
- feature gate new constructs and error variants depending on rustls
- add dicom_app_common=debug in verbose mode
@Enet4
Enet4 force-pushed the change/app-common/revise branch from 7539585 to 04129c0 Compare June 20, 2026 11:21
@Enet4 Enet4 added the A-app-common Library crate dicom-app-common label Jun 20, 2026
@qarmin

qarmin commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Can be findscu/src/query.rs and movescu/src/query.rs unified in app-common? This file is duplicated in this two places

Comment thread app-common/src/lib.rs Outdated
Comment thread app-common/src/lib.rs Outdated
Comment thread app-common/src/lib.rs Outdated
Comment thread app-common/src/lib.rs Outdated
Comment thread app-common/src/lib.rs Outdated
Comment thread app-common/src/lib.rs Outdated
@naterichman

Copy link
Copy Markdown
Contributor

Can be findscu/src/query.rs and movescu/src/query.rs unified in app-common? This file is duplicated in this two places

I don't think these belong in app-common, but they should be de-duplicated at some point. I have draft PR (#681) I need to get back to which adds structs for all the defined dimse messages. That should help de-duplicate somewhat.

After that, I think a few of us have been in conversations around adding a higher level module for dimse (see #781), issue discussion here

- remove typo from `PeerCertOption`  docs
- replace `use_value_delimiter` with `value_delimiter` in TlsOptions arguments
- move warning on ignore peer cert in client_config
@Enet4

Enet4 commented Jun 25, 2026

Copy link
Copy Markdown
Owner Author

Thank you for the feedback!

Regarding the query module, I would not be opposed to moving it to dicom-app-common, but there may be an even better place for it. As it turns out, describing small DICOM objects with this syntax is pretty useful, even outside DIMSE! Let us defer to the milestone after 0.10, though I agree that it should have a public API in its own right somewhere.

@Enet4
Enet4 merged commit fa673c6 into master Jun 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-app-common Library crate dicom-app-common A-tool Area: tooling C-storescp Crate: dicom-storescp C-storescu Crate: dicom-storescu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants