User trait

[xpromache]

Currently the CFDP "User" concept is handled via the struct User which is based on the interprocess crate implemented as Unix domain sockets on Unix and named pipes on Windows.

I propose to transform it into a trait similar with the transport trait, in which the current interprocess implementation is one implementation but users can define their own. I for instance want to implement a webservice to control the CFDP daemon.

The communication between the daemon and the implementers of the User trait could be done with another pair of channels. Doing that solves also the current problem of multiplexing the IPC socket with the transport channels in the daemon::manage_transactions: the problem now is that all the transport channels are part of a select but with a timeout of 0.5 mills in order to allow polling the IPC socket for new user messages - as it is difficult to multiplex channels with the IPC socket (it's true that switching to tokio/async will also allow solving this problem).

[mkolopanis]

I do like the idea of a channel to send primitives from the user program and not using interprocess. Would the user program still be accessible from a cli though?

[xpromache]

As I said in a default implementation of the User (let's call it IpcUser), it will be the same as now just that instead of the daemon reading directly from the IPC socket, the IpcUser will read from the socket and pass it to the daemon via channels.

But in other User implementations like mine, it would be accessible via web requests.

We may also consider a scenario where the thing is accessible both via IPC and via Web, I think once you switch to async the combinations will be limitless...

[mkolopanis]

Oh sorry I think I missed it in my first read. Yeah I like this idea.

[mkolopanis]

I'll work on rolling this out.

[xpromache]

Hi @mkolopanis, I'm sorry to come back to this issue only 3 days after I said it's ok :(

I was looking to build my "Web User" using axum. The problem I have is that the daemon launches the user into a new thread and all the requests are expected from that thread. In my case, I need to pass the requests from the Axum handlers (i.e. someone makes a put request via web, axum invokes a handler and from within that handler I need to pass the PutRequest to the daemon). I could work around it by making a dummy User that doesn't do anything that "extracting" the channel passed on by the daemon into some sort of outside variable which is then used into the web handlers.

So I was wondering if it wouldn't make sense for the Daemon that instead of taking a User as argument, to take just the channels to communicate with the user. So we wouldn't have a User trait per se, but whoever creates the daemon is free to do whatever with the other end of the channels.

The exiting IpcUser can create a new thread to be compatible with the old implementation. Even better, we could provide an example main implementation (currently lacking) in which we create a daemon in a new thread and the IpcUser in the main loop. To accompany this, we would also have a cfdp-cli application which can be used to issue put requests. These could be made in a different package (next to "cfdp-core")

[mkolopanis]

Don't worry about it! Really only find the issues when we try to use the code right?

Oh this is interesting. Then User is completely implementation specific and is free to behave as it likes. Just needs to communicate with the Daemon over the necessary channels. I think extra steps of "extracting" as you called it are unnecessary bloat and better to keep things simple. The inclusion of an example would go a long way this change too. Examples have always been on my mind just been concerned with getting (core) functionality for a mission first :sweat_smile:

This would also make the test code a bit simpler only needing keep track of one set of channels (currently uses two).

I'll re-open this till now and we can fix with your proposed change.

[xpromache]

Perfect, thank you very much!

I have another suggestion: currently this timeout of 500 microsecs makes the daemon consume 18% CPU when it is supposed to be completely idle (no transaction going on): https://github.com/ASU-cubesat/cfdp-rs/blob/main/cfdp-core/src/daemon.rs#L871

We could get rid of it if we used the channel disconnection as a termination signal. The while loop above can be replaced with a loop which will be broken when the user channel is disconnected (e.g. dropped from the user side).

The daemon can still make the signal in its Daemon::new method and distribute it to the transport (probably that could also be replaced by the same mechanism but that will be the next step).

[mkolopanis]

Yeah that's a good idea using the channel. Then once #6 is worked on it will be easier to just drop an async channel in to check for disconnection.

[xpromache]

Yeah that's a good idea using the channel. Then once #6 is worked on it will be easier to just drop an async channel in to check for disconnection.

I think with the standard channels it works as well. When you recv on the channel (after the select) an error indicates the channel has been disconnected. From what I understand that's the only reason you can get an error.

[mkolopanis]

I agree. I meant it should be easy to switch one out for the other.

[mkolopanis]

I've been chunking away at this and hit the point where the Indications to the user are becoming important to track. The first solution I thought of was to pass a Sender<TransactionIndication> to each transaction and let them populate their own indications into the channel. Where TransactoinIndication would be a new enum following CCSDS 727.0-B-5 §3.4. Thoughts?

[mkolopanis]

I think this would require re-working the current Sender<(TransactionID, TransmissionMode, Vec<MessageToUser>)> used to propagate user messages to also use the new enum. Might make it a little easier to read.

[xpromache]

I think this would require re-working the current Sender<(TransactionID, TransmissionMode, Vec<MessageToUser>)> used to propagate user messages to also use the new enum. Might make it a little easier to read.

Indeed, I think the current Sender<(TransactionID, TransmissionMode, Vec<MessageToUser>)> will disappear because the MessageToUser will be passed as part of the MetadataReceived indication.

[mkolopanis]

Great! I've started this overhaul on my flexible_user branch

On Fri, Mar 10, 2023, 17:21 Nicolae Mihalache @.***> wrote:

I think this would require re-working the current Sender<(TransactionID, TransmissionMode, Vec)> used to propagate user messages to also use the new enum. Might make it a little easier to read.

Indeed, I think the current Sender<(TransactionID, TransmissionMode, Vec)> will disappear because the MessageToUser will be passed as part of the MetadataReceived indication.

— Reply to this email directly, view it on GitHub https://github.com/ASU-cubesat/cfdp-rs/issues/16#issuecomment-1464716938, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACB7BLM54RNVH3D3FHGRHB3W3PARTANCNFSM6AAAAAAVJ36QMQ . You are receiving this because you modified the open/close state.Message ID: @.***>

[mkolopanis]

Another point that's come up is linking transactions IDs to input Put requests. The Implementers guide proposes two ways to facilitate this

• Block on each put request until the corresponding ID can be returned. This is basically what is already done, but requiring the Primitive sender to listen on the sender channel it sends.
• Use unique identifiers to map transaction IDs to requests. This would require generating Put Request primitives to have some form of unique identifier. Then the Transaction Indication would return ID and unique identifier. It would be up to the user to to decide how it would want to handle this. Whether to wait until the ID corresponding to the send identifier is returned or move on and map unique IDs to transaction IDs later.

Option 2 feels like it would make the primitive_rx on the Daemon a little simpler by only receiving UesrPrimitives. But offloads more work to the User later. But it somehow also feels more in the spirit of the software to me based on how the guides read.

[xpromache]

Another point that's come up is linking transactions IDs to input Put requests. The Implementers guide proposes two ways to facilitate this

• Block on each put request until the corresponding ID can be returned. This is basically what is already done, but requiring the Primitive sender to listen on the sender channel it sends.
• Use unique identifiers to map transaction IDs to requests. This would require generating Put Request primitives to have some form of unique identifier. Then the Transaction Indication would return ID and unique identifier. It would be up to the user to to decide how it would want to handle this. Whether to wait until the ID corresponding to the send identifier is returned or move on and map unique IDs to transaction IDs later.

Option 2 feels like it would make the primitive_rx on the Daemon a little simpler by only receiving UesrPrimitives. But offloads more work to the User later. But it somehow also feels more in the spirit of the software to me based on how the guides read.

Frankly for me option 1 feels better. The primitive_rx is not awfully complicated by having a Sender to return the newly created TransactionID to. Plus that one can return also errors in case the daemon finds something wrong with the Put Request. Option 2 involves creating and managing a new identifier, seems overkill for me (you have then to make sure that two senders will not come up with the same identifier).

One can also argue that the Sender sent together with the PutRequest is the unique identifier in the option 2. So options 1 and 2 are the same :) :)

[mkolopanis]

Fair points. Thanks for the feedback.

[mkolopanis]

removed user trait, to rely on implementation specific user in #30