Skip to content

457 early pre published alert pkp integration into sdpi a#465

Open
JavierEspina wants to merge 45 commits intomasterfrom
457-early-pre-published-alert-pkp-integration-into-sdpi-a
Open

457 early pre published alert pkp integration into sdpi a#465
JavierEspina wants to merge 45 commits intomasterfrom
457-early-pre-published-alert-pkp-integration-into-sdpi-a

Conversation

@JavierEspina
Copy link
Collaborator

@JavierEspina JavierEspina commented Jul 28, 2025

📑 Description

☑ Mandatory Tasks

The following aspects have been respected by the pull request assignee and at least one reviewer:

  • Changelog update (necessity checked and entry added or not added respectively)
    • Pull Request Assignee
    • Reviewer

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Jul 28, 2025

@ToddCooper, @d-gregorczyk, @PeterKranich - this PR is still in drafting and hence incomplete. But feel free to start looking at the changes before my summer holiday starts (on Aug 2).

@ToddCooper
Copy link
Collaborator

@JavierEspina - so remind me ...
Default is DIS and we have an option for DAS support. Yes?
Let me know when I should download a build to review especially the TF-1 SDPi-A stuff.

@JavierEspina
Copy link
Collaborator Author

@JavierEspina - so remind me ... Default is DIS and we have an option for DAS support. Yes? Let me know when I should download a build to review especially the TF-1 SDPi-A stuff.

Yes, by default a DIS. DAS is an option. When the DAS option is selected transactions DEV-48 and DEV-49 kick in (i.e., become mandatory)

You may want to download a build already to see how things are currently looking. There is no complete description of all steps in DEV-48 or -49 but the idea is there.

@PaulMartinsen
Copy link
Collaborator

Starting to look over adding in markup for the transactions I notice that in DEV-49, the alert provider is both a responder and initiator in the text but the alert consumer is listed only as an initiator. If the alert provider initiates this transaction wouldn't the alert consumer be at least a receiver as well?

@PaulMartinsen
Copy link
Collaborator

@JavierEspina , the markup for the Distributed Alarm System Option looks good to me. The markup doesn't support a transaction making multiple contributions (e.g., responder and initiator) currently so I'm going to have to add that to make the json export work.

What's the plan for merging this one? I could tackle this as a separate issue if you're in a hurry to merge it. Json exports would be incomplete in the meantime.

@JavierEspina
Copy link
Collaborator Author

Starting to look over adding in markup for the transactions I notice that in DEV-49, the alert provider is both a responder and initiator in the text but the alert consumer is listed only as an initiator. If the alert provider initiates this transaction wouldn't the alert consumer be at least a receiver as well?

You're right. It is not consistent right now. The first table of SDP-i A shows both actors and both initiator and responder. So does the diagram in the DEV-49 section. However the actor roles table in that section does not. I am going to correct this in my next commit...or at least try!

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Feb 20, 2026

@JavierEspina , the markup for the Distributed Alarm System Option looks good to me. The markup doesn't support a transaction making multiple contributions (e.g., responder and initiator) currently so I'm going to have to add that to make the json export work.

What's the plan for merging this one? I could tackle this as a separate issue if you're in a hurry to merge it. Json exports would be incomplete in the meantime.

My current expectation is to move this PR from draft to an actual PR (i.e. content complete for review for merging) within 1-2 weeks (maybe even 3). If that provides you with sufficient time to make that fix in this PR, I'd say: go ahead!

@PaulMartinsen
Copy link
Collaborator

Ok. 1 week might be a little challenging, but 2-3 should work. I'll make a start and can either include it or move to a separate issue if you are ready to merge first.

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Feb 27, 2026

Ok. 1 week might be a little challenging, but 2-3 should work. I'll make a start and can either include it or move to a separate issue if you are ready to merge first.

Additional remark: the lack of support for actors' multiple contributions (e.g., initiator and responder) does not only affect the JSON export but also the spec content. For instance, it makes the Contribution column of
"Table 2:3.49.2-1. Actor Roles [DEV-49]" incomplete and not quite consistent with the Role description and diagram below.

@PaulMartinsen
Copy link
Collaborator

@JavierEspina , I've added support for actors making multiple contributions to transactions now.

AsciiDoc:

[sdpi_transaction_actors]
--

[actor-id="somds_medical_alert_consumer",contribution=Initiator,contribution=Receiver]
Requests a <<vol1_spec_sdpi_a_actor_somds_medical_alert_provider>> to end a Distributed Alarm System.
Listens for notifications that the <<vol1_spec_sdpi_a_actor_somds_medical_alert_provider>> has ended
the Distributed Alarm System.


[actor-id="somds_medical_alert_provider",contribution=Responder,contribution=Initiator]
Listens for requests from a <<vol1_spec_sdpi_a_actor_somds_medical_alert_consumer>> to end the 
Distributed Alarm System and grants such requests. Alternatively ends the Distributed Alarm System 
on its own initiative and notifies the <<vol1_spec_sdpi_a_actor_somds_medical_alert_consumer>>.

--

Renders:
image

And to make a link, for the json artefacts, in the profile, for example:

sdpi_include_transaction::DEV-48[actor-id="somds_medical_alert_provider",responder=optional,initiator=optional]
sdpi_include_transaction::DEV-49[actor-id="somds_medical_alert_provider",responder="optional",initiator="optional"]

...and Distributed Alarm System Option, for example:

sdpi_include_transaction::DEV-48[actor-id=somds_medical_alert_provider,responder=required,initiator=required]
sdpi_include_transaction::DEV-49[actor-id=somds_medical_alert_provider,responder=required,initiator=required]

On merging, I noticed you had the alert consumer making a responder contribution in DEV-48, however it appears the consumer just receives, and doesn't respond to, notifications. So I changed the contribution to "Receiver" for consistency with, for example, DEV-49 and DEV-23, etc. Hopefully that's right!

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Mar 3, 2026

Thank you, @PaulMartinsen! The DEV-49 table looks good now.

What looks less good though, IMO, are the changes made to DEV-48. My intent was to leave DEV-48 as "unidirectional", that is the consumer initiates and the provider responds (and then they both "remain doing stuff" to keep the just-established DAS up and running). DEV-49 is a different story, both the consumer and provider can use that transaction to initiate a controlled ending of the DAS.

I am going to try to revert some of the changes to DEV-48. Wish me luck and no damage! 🤞

@PaulMartinsen
Copy link
Collaborator

You're welcome.

On DEV-48, I noticed the notification in the last step of your diagram is the same pattern as DEV-29, giving the actors in DEV-29 initiator, receiver contributions. DEV-27 would be a counter example though; there the provider can initiate the end of the subscription but doesn't have that role. I'm not really sure about this stuff so I'll leave to you!

image

@JavierEspina
Copy link
Collaborator Author

You're welcome.

On DEV-48, I noticed the notification in the last step of your diagram is the same pattern as DEV-29, giving the actors in DEV-29 initiator, receiver contributions. DEV-27 would be a counter example though; there the provider can initiate the end of the subscription but doesn't have that role. I'm not really sure about this stuff so I'll leave to you!

image

My understanding (hopefully correct... @ToddCooper ?) is that the role (initiator, responder/receiver) is for the entire transaction in general and not for every message exchange that makes up a transaction.

@JavierEspina JavierEspina marked this pull request as ready for review March 6, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Early "pre-published" Alert PKP Integration into SDPi-A

3 participants