Skip to content
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.

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

Instance Registration and Disposal fails #269

Open
FirebarSim opened this issue Oct 1, 2024 · 4 comments
Open

Instance Registration and Disposal fails #269

FirebarSim opened this issue Oct 1, 2024 · 4 comments

Comments

@FirebarSim
Copy link

When calling register_instance and passing in a sample of a type with defined keys the returned handle is a constant per run (i.e. run 1 always returns 123456789, run 2 always returns 987654321).

When calling dispose_instance_handle(instance_handle) a sample dispose is sent but for a different key than that of any of the registered instances.

When calling dispose_instance(sample) a sample dispose is sent but for a different key than that of the sample.

The samples being registered are OMG OARIS.

@FirebarSim FirebarSim changed the title Instance Registration and DIsposal Instance Registration and Disposal fails Oct 1, 2024
@eboasson
Copy link
Contributor

eboasson commented Oct 3, 2024

Hi @FirebarSim, this:

When calling register_instance and passing in a sample of a type with defined keys the returned handle is a constant per run (i.e. run 1 always returns 123456789, run 2 always returns 987654321).

fits with how Cyclone does things. Registering the instance is the main point at which it generates an instance handle, and they are generated by running a 64-bit cipher in counter mode with a random key, so to a good approximation each and every time you need an instance handle in whatever context, you get a new unique number within a single process and that number moreover is highly likely to be unique over the lifetime of the DDS system.

These observations:

When calling dispose_instance_handle(instance_handle) a sample dispose is sent but for a different key than that of any of the registered instances.

When calling dispose_instance(sample) a sample dispose is sent but for a different key than that of the sample.

do worry me a bit. I tried a simple experiment using the following:

from dataclasses import dataclass
from cyclonedds.domain import DomainParticipant
from cyclonedds.pub import DataWriter
from cyclonedds.sub import DataReader, InvalidSample
from cyclonedds.topic import Topic
from cyclonedds.core import Qos, Policy
import cyclonedds.idl as idl
from cyclonedds.idl.annotations import key
import cyclonedds.idl.types as types

from time import sleep

@dataclass
class S(idl.IdlStruct, typename="S"):
    s: str
    key("s")
    v: int

dp = DomainParticipant(0)
tp = Topic(dp, "S", S, qos=Qos(Policy.History.KeepAll))
dw = DataWriter(dp, tp)
dr = DataReader(dp, tp)

ih0 = dw.register_instance(S("aap", 0))
ih1 = dw.register_instance(S("noot", 0))
print(f"ih0={ih0} ih1={ih1}")

b = 0
while True:
    if b % 2 == 0:
        dw.write(S("aap", b))
        dw.write(S("noot", b))
    else:
        dw.dispose(S("aap", b))
        dw.dispose_instance_handle(ih1)
    xs = dr.take(10)
    for x in xs:
        if isinstance(x, InvalidSample):
            print("Took invsample ", x.key_sample, x.sample_info.instance_handle)
        else:
            print("Took ", x, x.sample_info.instance_handle)
    sleep(1)
    b = b+1

it behaves like I expect: it prints two instances handles at the beginning, and then afterward those are the two instance handles that show up time and time again. The key value in the dispose_instance_handle case is always "noot", as expected.

Moreover, if I start a second copy with the writing/disposing commented out, I also get that result. The instance handles are of course different from the first copy (different process → different key → unique instance handles), but the contents are as expected and the instance handles in the sample infos line up with the output of register_instance in the second copy.

Perhaps there is something in the OARIS types that cause it to misbehave. Simply fiddling with the type of S the values should then trigger it. Perhaps you can have a look at what the key type is for the topics you are seeing the problem, and see if that makes my little program reproduce the misbehaviour?

@FirebarSim
Copy link
Author

The OARIS Sensor Track is keyed on subsystem id and sensor track id, here's the python class for it (aren't open standards nice?)
image

In this case I am just generating some tracks from some ADS-B data, here are a few of the reports that I am getting:
image
Generated by this code:
image

On disposal the following is reported
image
By this code
image

I believe that I am doing this correctly, but I could of course be completely cocking it up. Running your code above on my VM works as I would expect. Altering the class to take the Subsystem ID and Sensor Track ID from OARIS also works fine. Doing the same with a pair of OARIS tracks (with only the sensor track ID different) returns the same instance handle.

@eboasson
Copy link
Contributor

eboasson commented Oct 7, 2024

Found it! It is a confusion inside the python binding between samples and key values. In the DDS API spec register_instance and dispose take a sample, but the only fields that matter are the key fields.

The Cyclone Python binding however serializes it as a normal sample1, then treats it as-if it is a serialized key value. In my first attempt at reproducing it, the key is at the beginning of the serialized data and it all seems fine. In the OARIS data type, however, it is somewhere in the middle and at the end ...

If I change my program to have the key as a the second field, e.g.:

from dataclasses import dataclass
from cyclonedds.domain import DomainParticipant
from cyclonedds.pub import DataWriter
from cyclonedds.sub import DataReader, InvalidSample
from cyclonedds.topic import Topic
from cyclonedds.core import Qos, Policy
import cyclonedds.idl as idl
from cyclonedds.idl.annotations import key
import cyclonedds.idl.types as types

from time import sleep

@dataclass
class S(idl.IdlStruct, typename="S"):
    v: int
    k: int
    key("k")

dp = DomainParticipant(0)
tp = Topic(dp, "S", S, qos=Qos(Policy.History.KeepAll))
dw = DataWriter(dp, tp)
dr = DataReader(dp, tp)

ih0 = dw.register_instance(S(0, 1))
ih1 = dw.register_instance(S(0, 2))
print(f"ih0={ih0} ih1={ih1}")

b = 0
while True:
    if b % 2 == 0:
        dw.write(S(b, 1))
        dw.write(S(b, 2))
    else:
        dw.dispose(S(b, 1))
        dw.dispose_instance_handle(ih1)
    xs = dr.take(10)
    for x in xs:
        if isinstance(x, InvalidSample):
            print("Took invsample ", x.key_sample, x.sample_info.instance_handle)
        else:
            print("Took ", x, x.sample_info.instance_handle)
    sleep(1)
    b = b+1

it misbehaves in the same way as your program.

While I know the root cause and have a simple proof-of-concept fix — a.k.a. "a hack" — that makes register_instance work, I need some more time to figure out the proper fix.

Footnotes

  1. Serialization is by far the easiest way to transition between Python and C.

@FirebarSim
Copy link
Author

Well that is great news that there is a definite solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants