Hardcoded carrier names

[drdanz]

YARP is supposed to be agnostic about which carrier is currently used, but in some places the carrier name is checked. For example:

yarp/src/libYARP_OS/src/NameServer.cpp

Lines 367 to 377 in 290545d

	if (carrier=="local"||carrier=="shmem") {
	if (srcRec.getProp("ips") == destRec.getProp("ips")) {
	if (carrier=="local") {
	if (srcRec.getProp("process") != destRec.getProp("process")) {
	ok = false;
	}
	}
	} else {
	ok = false;
	}
	}

In this example local and shmem are checked explicitly, but this will not work for any other carrier.
I think we should add 2 methods to the Carrier interface that allows to perform this check, something like sameProcessOnly(), and sameSystemOnly() both returning to false if not explicitly implemented in the carrier (Any suggestion on the naming is welcome).

I'm not really sure if this is ok though, since this part will run (I think) on the yarpserver, and it could be running on a machine where the carrier is not available. For example imagine a that we had a carrier that runs only on windows, and the yarp server is running on linux... How can we handle this?

There are other hardcoded carrier names (mostly tcp), we should also investigate those.

[Nicogene]

Unfortunately there is not the 😭 reaction. 😬

I totally agree to remove these hardcoded checks and define functions to the carrier level.

In another carrier that I worked to, I do this check inside the carrier itself during the initial handshake.
So if two ports are not on the same machine the connect fails, I don't know if it is possible to do the same for the local and shmem carrier.

[lornat75]

what about:

interProcess()/intraProcess() and interHost()/intraHost()

[drdanz]

Sounds good, but I always have doubts about intra vs inter 😣

[drdanz]

After a quick search, I'd say that intraProcess() and intraHost() are the right ones...

[drdanz]

I think we should also specify in the documentation the exact meaning of these, because they are already confusing to me...

intraProcess() should be "works intra process only" (i.e. only local) or "works intra process" (i.e. all carriers)
intraHost() should be "works intra host only" (i.e. shmem) or "works intra host" (i.e. all carriers, except local)

[lornat75]

Right. Is there the need to also specify that a protocol is interHost ?

[drdanz]

// Carrier.h

/**
 * Returns true if this carrier works only between 2 ports on the same process
 */
bool interProcessOnly() = 0; // { return false; } in AbstractCarrier, reimplemented in local carrier

/**
 * Returns true if this carrier works only between 2 ports on the same host
 */
bool interHostOnly() = 0; // { return false; } in AbstractCarrier, reimplemented in shmem carrier

Or perhaps something like this would be better

enum class Foo
{
    None = 0,
    IntraProcess,
    IntraHost,
    Any
};

Foo aSignificantMethodName()
{
    return Foo::Any;
}

or also (over-engineering?)

enum class Foo
{
    None = 0,
    WorksIntraProcess = 0x0001,
    WorksIntraHost = 0x0002,
    WorksInterHost = 0x0004,

    IntraProcessOnly = WorksIntraProcess,
    IntraHostOnly = WorksIntraProcess | WorksIntraHost,
    Any = WorksIntraProcess | WorksIntraHost | WorksInterHost
};

Foo aSignificantMethodName()
{
    return Foo::Any;
}

[barbalberto]

Personally, I'd prefer the first option. More clear and I get directly the info I want.
Btw, I think the name should be intraXXX instead of interXXX.

Inter- is a prefix that means between two groups, and intra- is a prefix which means within or inside one group.

Or maybe SameProcessOnly SameHostOnly 😄

[lornat75]

The second option has the advantage that it makes the options mutually exclusive, whereas in the first case you have to explicitly make sure that a carrier does not return true on both.

[barbalberto]

I was wandering exactly about that.
A intra-host carrier could also be intra-process, in theory, while it is not true the other way around.

[drdanz]

As discussed with @lornat75, we are proposing to use this naming:

enum class Foo
{
    None = 0,
    
    WorksIntraProcess = 0x0001,
    WorksIntraHost    = 0x0002,
    WorksInterHost    = 0x0004,

    IntraProcess = WorksIntraProcess
    IntraHostOnly = WorksIntraHost,
    IntraHost = WorksIntraProcess | WorksIntraHost,
    InterHostOnly = WorksInterHost,
    InterHost =  WorksIntraProcess | WorksIntraHost | WorksInterHost
};