Skip to content

DRF Expansion #24

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.

Sign up for GitHub

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

Open
rneswold wants to merge 6 commits into
base: main
Choose a base branch
from
Open

DRF Expansion #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
407ce27
:memo: document some messages
rneswold Jul 18, 2025
1af5c05
:construction: enhance DRF messages
rneswold Jul 18, 2025
96ba14d
:bug: field names need to be different
rneswold Jul 18, 2025
a9bac15
:recycle: add time to arm and stop events
rneswold Jul 18, 2025
5cf99ab
:recycle: Adjust how to support post trigger
rneswold Jul 21, 2025
d73ab7b
:bug: move `never` to `ArmEvent`
rneswold Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions proto/controls/common/v1/drf.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
// These messages are used to encode data acquisition requests. The
// original organization of these messages was inspired by the Data
// Request Format document, which encoded this information in strings
// of text. These messages have been expanded upon and now can express
// more forms of acquisition than DRF was able.

syntax = "proto3";

package common.drf;
Expand Down Expand Up @@ -25,14 +31,18 @@ message DataChannel {

message DRFs {
repeated DataChannel data_channel = 1;
optional common.event.Event event = 2;
optional common.sources.Source source = 3;
common.event.ArmEvent arm = 2;
common.event.SampleEvent sample = 3;
common.event.StopEvent stop = 4;
optional common.sources.Source source = 5;
}
Copy link

@KitFieldhouse KitFieldhouse Jul 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What I have implemented has slightly different syntax here. Instead of the fields: arm, sample, and stop, I essentially use four fields; sample, arm, trigger, and stop (though I call them other names and I didn't implement them as a proto).

The semantics I had in mind is that sample is what is continuously generating the data stream and the other fields act as a "valve" that controls if that data stream is emitted to the client.

I think this allows for slightly more flexible acquisition triggering. For example, we could be sampling 1440Hz data and trigger on the first 1D after a 12 up until the extraction event by setting the fields like so:

sample: Periodic 1440Hz
arm: 0x12
trigger: 0x1D
stop: 0X1F

Or sampling 1440Hz ftp only on 1Ds that are accompanied by the HEP enable event 52 like so:

sample: 1440Hz
arm: 0x1D
trigger: 0x52
stop: 0x1F

Essentially, compared to this change, separating sample and trigger allows for an additional clock condition (or other condition, like a state change) that needs to be cleared before sending data. However, one could imagine allowing for arbitrarily long sequences of clock events (or other conditions) that need to happen in a sequence before allowing data to be emitted, so having this additional condition might not really buy us much.

Copy link
Contributor

@beauremus beauremus Jul 21, 2025

Choose a reason for hiding this comment

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

Does this request "feel" good to you?

sample: Periodic 1440Hz
arm: 0x12
trigger: 0x1D
stop: 0X1F

request = DRF(
    data_channel=DataChannel(
        name="device.status",
        range=Range(full_range=google.protobuf.Empty()),
        field=Field(status=Field.Flavor.ON)
    ),
    sample=SampleEvent(
        periodic=SampleEvent.Periodic(
            # This stinks and could be an issue with how this is "rounded"
            period=google.protobuf.Duration(nanos=math.trunc(1/1440*1_000_000_000)),
            immediate=True
        )
    ),
    arm=ArmEvent(
        clock=common.event.Clock(
            event_number=int("12", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    ),
    trigger=TriggerEvent(
        clock=common.event.Clock(
            event_number=int("1D", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    ),
    stop=StopEvent(
        clock=common.event.Clock(
            event_number=int("1F", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    )
)

sample: 1440Hz
arm: 0x1D
trigger: 0x52
stop: 0x1F

request = DRF(
    data_channel=DataChannel(
        name="device.status",
        range=Range(full_range=google.protobuf.Empty()),
        field=Field(status=Field.Flavor.ON)
    ),
    sample=SampleEvent(
        periodic=SampleEvent.Periodic(
            # This stinks and could be an issue with how this is "rounded"
            period=google.protobuf.Duration(nanos=math.trunc(1/1440*1_000_000_000)),
            immediate=True
        )
    ),
    arm=ArmEvent(
        clock=common.event.Clock(
            event_number=int("1D", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    ),
    trigger=TriggerEvent(
        clock=common.event.Clock(
            event_number=int("52", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    ),
    stop=StopEvent(
        clock=common.event.Clock(
            event_number=int("1F", 16),
            event_type=common.event.Clock.EventType.HARDWARE
        )
    )
)

I think this would require something like:

message TriggerEvent {
    oneof event {
        google.protobuf.Empty immediate = 1; # This is the default, so a no-op
	Clock clock = 2;
	State state = 3;
	google.protobuf.Duration delay = 4;
    }
}

Copy link
Contributor Author

@rneswold rneswold Jul 21, 2025

Choose a reason for hiding this comment

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

I like it!

We may be able to complicate/enhance it further in order to support more complex trigger conditions. It doesn't guarantee all front-ends will be ale to support it. But DPM should be able to simulate to conditions that the FE can't. For instance, it could ask for 1440Hz data and do all the filtering itself based on monitoring the clock system.

Copy link
Contributor

@beauremus beauremus Jul 21, 2025

Choose a reason for hiding this comment

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

My understanding from @KitFieldhouse is that is what KitDPM does.
I think we should pull that into a service, but that's nitpicking.

Copy link
Contributor

@beauremus beauremus Jul 21, 2025

Choose a reason for hiding this comment

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

Speaking to "complex trigger conditions" I was considering posting this and decided not to. Does this align with what you were thinking?

// Represents a single condition within a compound event (either a Clock or a State).
message EventCondition {
    oneof condition_type {
        Clock clock = 1;
        State state = 2;
    }
}

// Defines a compound event by combining multiple EventConditions with logical operators.
message CompoundEvent {
    enum Operator {
        AND = 0;      // All conditions must be met (e.g., all clocks fire near-simultaneously)
        OR = 1;       // Any one condition must be met
        SEQUENCE = 2; // Conditions must be met in the specified order
    }

    Operator operator = 1;
    repeated EventCondition conditions = 2; // List of individual event conditions
}

Copy link
Contributor Author

@rneswold rneswold Jul 22, 2025

Choose a reason for hiding this comment

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

I guess I need to know how a "user trigger" works. Is it a hardware signal that a user can activate? Or a gRPC request? Setting a state device would be similar complexity as the latter.

Copy link
Contributor Author

@rneswold rneswold Jul 22, 2025

Choose a reason for hiding this comment

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

RE: your LLM examples. Those are fairly messy for a developer to use. But I would imagine that our libraries would provide a cleaner interface and would generate those messages.

Copy link
Contributor

@beauremus beauremus Jul 22, 2025

Choose a reason for hiding this comment

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

I guess I need to know how a "user trigger" works. Is it a hardware signal that a user can activate? Or a gRPC request? Setting a state device would be similar complexity as the latter.

I had assumed the latter. @KitFieldhouse did you mean something different?

Copy link

@KitFieldhouse KitFieldhouse Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For this architecture yeah it would be a gRPC request.

The idea was that, when submitting a request with a user trigger, you would get some unique ID for that trigger that you could then use with another API endpoint to satisfy that trigger.

The implementation I had originally had in mind might not fit very well with the current and planned daq architecture, but the idea was for the API route that fires the user trigger to be a simple HTTP POST with an empty body and a query parameter equal to the "User Trigger ID". By sending that request the trigger is then satisfied. The idea is that this trigger then essentially has a unique URL that can then be used for firing that trigger, either through a browser or with a command line utility like curl.

For the gRPC architecture, maybe this would be a new API endpoint that takes a "User Trigger ID" and then fires that trigger on the DPMs or returns some sort of error if no user trigger matches that ID?

Copy link
Contributor Author

@rneswold rneswold Jul 23, 2025

Choose a reason for hiding this comment

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

I feel like this is slightly more formal than state events but does, essentially, the same thing. Except the DPMs already listen to state multicasts.


message DRF {
DataChannel data_channel = 1;
optional common.event.Event event = 2;
optional common.sources.Source source = 3;
common.event.ArmEvent arm = 2;
common.event.SampleEvent sample = 3;
common.event.StopEvent stop = 4;
optional common.sources.Source source = 5;
}

message DRFList {
Expand Down Expand Up @@ -87,6 +97,12 @@ message Field {
}
}

// Specifies how much of a device's data should be returned. For
// simple devices that only return primitive types, the `full_range`
// should be used. The control system also supports "array devices"
// (a.k.a. waveform devices). For those devices, the `ArrayRange` can
// be used to return a portion of the device's full value.

message Range {
oneof range {
google.protobuf.Empty full_range = 1;
Expand Down
70 changes: 46 additions & 24 deletions proto/controls/common/v1/event.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,70 @@ syntax = "proto3";
package common.event;

import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto";
import "google/protobuf/empty.proto";

message Event {
message Clock {
int32 event_number = 1;
EventType event_type = 2;
optional google.protobuf.Duration delay = 3;
}
// Specifies a clock event with an optional delay to wait after the
// event fires.

message Clock {
enum EventType {
HARDWARE = 0;
SOFTWARE = 1;
EITHER = 2;
}

message Periodic {
bool continuous = 1;
google.protobuf.Duration period = 2;
bool immediate = 3;
int32 event_number = 1;
EventType event_type = 2;
Copy link
Contributor

@beauremus beauremus Jul 18, 2025

Choose a reason for hiding this comment

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

We should deprecate this. Can we mark it as deprecated?

I think the default is good, which is good news!

If the default value is not specified for an optional element, a type-specific default value is used instead: for strings, the default value is the empty string. For bools, the default value is false. For numeric types, the default value is zero. For enums, the default value is the first value listed in the enum's type definition. This means care must be taken when adding a value to the beginning of an enum value list.

Copy link
Contributor Author

@rneswold rneswold Jul 23, 2025

Choose a reason for hiding this comment

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

What do you feel is deprecated?

Copy link
Contributor

@beauremus beauremus Jul 23, 2025

Choose a reason for hiding this comment

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

The event_type. We should hardware-trigger everything.

optional google.protobuf.Duration delay = 3;
}

message State {
string state_name = 1;
optional google.protobuf.Duration delay = 2;

oneof expression {
int32 equal = 3;
int32 not_equal = 4;
int32 less_than = 5;
int32 less_than_or_equal = 6;
int32 greater_than = 7;
int32 greater_than_or_equal = 8;
google.protobuf.Empty any_value = 9;
}
}

message ArmEvent {
oneof event {
google.protobuf.Empty never = 1;
google.protobuf.Empty immediate = 2;
Clock clock = 3;
State state = 4;
google.protobuf.Timestamp time = 5;
}
}

message State {
string state_name = 1;
optional google.protobuf.Duration delay = 2;

oneof expression {
int32 equal = 3;
int32 not_equal = 4;
int32 less_than = 5;
int32 less_than_or_equal = 6;
int32 greater_than = 7;
int32 greater_than_or_equal = 8;
google.protobuf.Empty any_value = 9;
}
message SampleEvent {
message Periodic {
google.protobuf.Duration period = 1;
bool immediate = 2;
Copy link
Contributor

@beauremus beauremus Jul 18, 2025

Choose a reason for hiding this comment

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

I think immediate is a confusing feature. We should do some documentation to explain to users what to expect.

Copy link
Contributor Author

@rneswold rneswold Jul 21, 2025

Choose a reason for hiding this comment

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

Maybe we could come up with a better name.

Copy link
Contributor Author

@rneswold rneswold Jul 21, 2025

Choose a reason for hiding this comment

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

no_delay?

Copy link
Contributor

@beauremus beauremus Jul 22, 2025

Choose a reason for hiding this comment

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

Yeah. I like no_delay.

Copy link
Contributor Author

@rneswold rneswold Jul 23, 2025

Choose a reason for hiding this comment

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

Or add_immediate?

Copy link
Contributor

@beauremus beauremus Jul 23, 2025

Choose a reason for hiding this comment

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

I don't think add_immediate is clear.

}

oneof event {
google.protobuf.Empty immediate = 1;
Periodic periodic = 2;
Clock clock = 3;
State state = 4;
google.protobuf.Empty never = 5;
}
int32 count = 5;
}

message StopEvent {
oneof event {
google.protobuf.Empty never = 1;
google.protobuf.Duration delay = 2;
Clock clock = 3;
State state = 4;
google.protobuf.Timestamp time = 5;
}
}