`find_topic` returning incorrect `TopicKind`

[sevnx]

Summary

The Topic element returned by the find_topic method in the DataParticipant incorrectly reports the TopicKind.

Reproducible example

use rustdds::{DomainParticipant, Keyed, QosPolicies};
use serde::{Deserialize, Serialize};
use std::time::Duration;

#[derive(Serialize, Deserialize, Clone, Debug)]
struct HelloWorldData {
    pub user_id: i32,
    pub message: String,
}

impl Keyed for HelloWorldData {
    type K = i32;

    fn key(&self) -> Self::K {
        self.user_id
    }
}

fn main() {
    let dp = DomainParticipant::new(0).expect("Failed to create DomainParticipant");

    let topic = dp
        .create_topic(
            "HelloWorld".to_owned(),
            "HelloWorldData".to_owned(),
            &QosPolicies::qos_none(),
            rustdds::TopicKind::WithKey,
        )
        .unwrap();

    let publisher = dp.create_publisher(&QosPolicies::qos_none()).unwrap();

    publisher
        .create_datawriter_cdr::<HelloWorldData>(&topic, None)
        .unwrap();

    let found_topic = dp
        .find_topic("HelloWorld", Duration::from_secs(1))
        .unwrap()
        .unwrap();

    assert_eq!(topic.kind(), found_topic.kind());
}

This example currently panics

thread 'main' panicked at src/main.rs:45:5:
assertion `left == right` failed
  left: WithKey
 right: NoKey

Analysis

The problem stems from the find_topic_in_discovery_db method in src/dds/particpant.rs:1330

let build_topic_fn = |d: &DiscoveredTopicData| {
      let qos = d.topic_data.qos();
      let topic_kind = match d.topic_data.key {
        Some(_) => TopicKind::WithKey,
        None => TopicKind::NoKey,
      };
      let name = d.topic_name().clone();
      let type_desc = d.topic_data.type_name.clone();
      self.create_topic(domain_participant_weak, name, type_desc, &qos, topic_kind)
    };

Here the decision whether the topic is keyed is done based on the key field of the DiscoveredTopicData, however that seems to be incorrect as I couldn't find a place in the specification where it would be interpreted as such. What's more, implementation of to_topic_data for pub/sub always set it to None.

I also checked why the Topic needs the information about being Keyed or not, and it is only used for checks when creating a DataReader, to not be able to create a Keyed DataReader for a NonKeyed Topic. (those checks are not present for DataWriter).

Proposed solution

A possible solution would be to find another way to identify if a Topic is keyed or not, would you have an idea on how to do so ?

Other than that, shouldn't a similar check (Keyed / NonKeyed w/ Topic) be added to the DataWriter?

[jhelovuo]

Here the decision whether the topic is keyed is done based on the key field of the DiscoveredTopicData, however that seems to be incorrect as I couldn't find a place in the specification where it would be interpreted as such. What's more, implementation of to_topic_data for pub/sub always set it to None.

Yes, your analysis seems correct. The current code is likely based on a misunderstanding of the specification. Thank you for pointing this out.

A possible solution would be to find another way to identify if a Topic is keyed or not, would you have an idea on how to do so ?

By a quick search through RTPS specification v2.5, it shows that there is a data type TopicKind_t, which is an enumeration with values NO_KEY and WITH_KEY. The RTPS Readers and Writers should use this type to record the kind of topic they are connected to.

This is something that is done differently in RustDDS: There are different Reader/Writer object types for with_key and no_key topics, and therefore no need for runtime bookkeeping.

Inspection of the Discovery data structures in the RTPS specification seems to indicate that the TopicKind_t type is not communicated at all over Discovery. It seems that is it expected that all the particiapnts have somehow pre-agreed the TopicKind for each topic, similar to agreeing the structure of each data type. The basic version of the Discovery process communicates only the data type name for a Topic, but not its structure.

In light of this, it may be that the entire .kind() call may be nonsensical, since it is not possible to get that information from Discovery. Then the correct solution would be to just remove the kind functionality from Topics.

Other than that, shouldn't a similar check (Keyed / NonKeyed w/ Topic) be added to the DataWriter?

Yes, DataWriter should be symmetrical w.r.t. the DataReader.