Revert "Return sample notifying changes on instance state (#5943)" (#5968)

This reverts commit 0b18cfe.

Signed-off-by: Jorge J. Perez <[email protected]>

Author: Blast545

src/cpp/fastdds/subscriber/DataReaderImpl.cpp

@@ -388,28 +388,6 @@ bool DataReaderImpl::wait_for_unread_message(
 void DataReaderImpl::set_read_communication_status(
         bool trigger_value)
 {
-    if (trigger_value)
-    {
-        auto user_reader = user_datareader_;
-
-        //First check if we can handle with on_data_on_readers
-        SubscriberListener* subscriber_listener =
-                subscriber_->get_listener_for(StatusMask::data_on_readers());
-        if (subscriber_listener != nullptr)
-        {
-            subscriber_listener->on_data_on_readers(subscriber_->user_subscriber_);
-        }
-        else
-        {
-            // If not, try with on_data_available
-            DataReaderListener* listener = get_listener_for(StatusMask::data_available());
-            if (listener != nullptr)
-            {
-                listener->on_data_available(user_reader);
-            }
-        }
-    }
-
     StatusMask notify_status = StatusMask::data_on_readers();
     subscriber_->user_subscriber_->get_statuscondition().get_impl()->set_status(notify_status, trigger_value);
 
@@ -741,6 +719,11 @@ ReturnCode_t DataReaderImpl::read_or_take_next_sample(
         return RETCODE_NOT_ENABLED;
     }
 
+    if (history_.getHistorySize() == 0)
+    {
+        return RETCODE_NO_DATA;
+    }
+
 #if HAVE_STRICT_REALTIME
     auto max_blocking_time = std::chrono::steady_clock::now() +
             std::chrono::microseconds(::TimeConv::Time_t2MicroSecondsInt64(qos_.reliability().max_blocking_time));
@@ -935,6 +918,25 @@ void DataReaderImpl::InnerDataReaderListener::on_data_available(
 
     if (data_reader_->on_data_available(writer_guid, first_sequence, last_sequence))
     {
+        auto user_reader = data_reader_->user_datareader_;
+
+        //First check if we can handle with on_data_on_readers
+        SubscriberListener* subscriber_listener =
+                data_reader_->subscriber_->get_listener_for(StatusMask::data_on_readers());
+        if (subscriber_listener != nullptr)
+        {
+            subscriber_listener->on_data_on_readers(data_reader_->subscriber_->user_subscriber_);
+        }
+        else
+        {
+            // If not, try with on_data_available
+            DataReaderListener* listener = data_reader_->get_listener_for(StatusMask::data_available());
+            if (listener != nullptr)
+            {
+                listener->on_data_available(user_reader);
+            }
+        }
+
         data_reader_->set_read_communication_status(true);
     }
 }
@@ -1174,10 +1176,7 @@ void DataReaderImpl::update_subscription_matched_status(
 
     if (count_change < 0)
     {
-        if (history_.writer_not_alive(status.remoteEndpointGuid))
-        {
-            set_read_communication_status(true);
-        }
+        history_.writer_not_alive(status.remoteEndpointGuid);
         try_notify_read_conditions();
     }
 }
@@ -1492,10 +1491,7 @@ LivelinessChangedStatus& DataReaderImpl::update_liveliness_status(
 {
     if (0 < status.not_alive_count_change)
     {
-        if (history_.writer_not_alive(iHandle2GUID(status.last_publication_handle)))
-        {
-            set_read_communication_status(true);
-        }
+        history_.writer_not_alive(iHandle2GUID(status.last_publication_handle));
         try_notify_read_conditions();
     }
 

src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp

@@ -180,18 +180,6 @@ struct ReadTakeCommand
             ++it;
         }
 
-        // Check if there is a state notification sample available
-        if (!finished_ && instance_->second->has_state_notification_sample)
-        {
-            // Add sample and info to collections
-            bool deserialization_error = false;
-            bool added = add_sample(nullptr, deserialization_error);
-            if (added && take_samples)
-            {
-                instance_->second->has_state_notification_sample = false;
-            }
-        }
-
         if (current_slot_ > first_slot)
         {
             history_.instance_viewed_nts(instance_->second);
@@ -406,34 +394,7 @@ struct ReadTakeCommand
         }
 
         SampleInfo& info = sample_infos_[current_slot_];
-        const DataReaderInstance& instance = *instance_->second;
-        if (item)
-        {
-            generate_info(info, instance, item);
-        }
-        else
-        {
-            fastdds::rtps::Time_t current_time;
-            fastdds::rtps::Time_t::now(current_time);
-
-            info.sample_state = NOT_READ_SAMPLE_STATE;
-            info.instance_state = instance.instance_state;
-            info.view_state = instance.view_state;
-            info.disposed_generation_count = instance.disposed_generation_count;
-            info.no_writers_generation_count = instance.no_writers_generation_count;
-            info.sample_rank = 0;
-            info.generation_rank = 0;
-            info.absolute_generation_rank = 0;
-            info.source_timestamp = current_time;
-            info.reception_timestamp = current_time;
-            info.instance_handle = instance_->first;
-            info.publication_handle = HANDLE_NIL;
-
-            info.sample_identity = rtps::SampleIdentity{};
-            info.related_sample_identity = rtps::SampleIdentity{};
-
-            info.valid_data = false;
-        }
+        generate_info(info, *instance_->second, item);
     }
 
     bool check_datasharing_validity(

src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp

@@ -661,7 +661,7 @@ void DataReaderHistory::check_and_remove_instance(
 {
     DataReaderInstance* instance = instance_info->second.get();
 
-    if (instance->cache_changes.empty() && (false == instance->has_state_notification_sample))
+    if (instance->cache_changes.empty())
     {
         if (InstanceStateKind::ALIVE_INSTANCE_STATE != instance->instance_state &&
                 instance->alive_writers.empty() &&
@@ -887,24 +887,13 @@ bool DataReaderHistory::update_instance_nts(
     return ret;
 }
 
-bool DataReaderHistory::writer_not_alive(
+void DataReaderHistory::writer_not_alive(
         const GUID_t& writer_guid)
 {
-    bool ret_val = false;
-
     for (auto& it : instances_)
     {
-        bool had_notification_sample = it.second->has_state_notification_sample;
         it.second->writer_removed(counters_, writer_guid);
-        if (it.second->has_state_notification_sample && !had_notification_sample)
-        {
-            // Mark instance as data available
-            data_available_instances_[it.first] = it.second;
-            ret_val = true;
-        }
     }
-
-    return ret_val;
 }
 
 StateFilter DataReaderHistory::get_mask_status() const noexcept

src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp

@@ -349,15 +349,7 @@ class DataReaderHistory : public eprosima::fastdds::rtps::ReaderHistory
     bool update_instance_nts(
             CacheChange_t* const change);
 
-    /*!
-     * @brief Inform the history that a writer should be considered as not alive.
-     *
-     * @param [in] writer_guid GUID of the writer that should be considered as not alive.
-     *
-     * @return true if a state notification sample was added to at least one instance.
-     *         This would mean that DATA_AVAILABLE status (and listener) shall be notified.
-     */
-    bool writer_not_alive(
+    void writer_not_alive(
             const fastdds::rtps::GUID_t& writer_guid);
 
     void check_and_remove_instance(

src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp

@@ -58,8 +58,6 @@ struct DataReaderInstance
     int32_t disposed_generation_count = 0;
     //! Current no_writers generation of the instance
     int32_t no_writers_generation_count = 0;
-    //! Whether the instance has a state notification sample available
-    bool has_state_notification_sample = false;
 
     DataReaderInstance(
             const eprosima::fastdds::ResourceLimitedContainerConfig& changes_allocation,
@@ -134,7 +132,6 @@ struct DataReaderInstance
                 break;
         }
 
-        has_state_notification_sample = false;
         return ret_val;
     }
 
@@ -160,7 +157,6 @@ struct DataReaderInstance
                 if (alive_writers.empty() && (InstanceStateKind::ALIVE_INSTANCE_STATE == instance_state))
                 {
                     instance_state = InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE;
-                    has_state_notification_sample = true;
                 }
                 if (ALIVE_INSTANCE_STATE == instance_state)
                 {
@@ -295,7 +291,6 @@ struct DataReaderInstance
             {
                 instance_state = InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE;
                 counters_update(counters.instances_alive, counters.instances_no_writers, counters, false);
-                has_state_notification_sample = true;
             }
 
             ret_val = true;

test/blackbox/api/dds-pim/PubSubReader.hpp

@@ -35,7 +35,6 @@
 #include <Windows.h>
 #endif // _MSC_VER
 #include <fastdds/dds/builtin/topic/ParticipantBuiltinTopicData.hpp>
-#include <fastdds/dds/common/InstanceHandle.hpp>
 #include <fastdds/dds/core/condition/GuardCondition.hpp>
 #include <fastdds/dds/core/condition/StatusCondition.hpp>
 #include <fastdds/dds/core/condition/WaitSet.hpp>
@@ -1786,10 +1785,7 @@ class PubSubReader
 
         if (eprosima::fastdds::dds::RETCODE_OK == datareader_->take(data_seq, info_seq))
         {
-            if (info_seq[0].publication_handle != eprosima::fastdds::dds::HANDLE_NIL)
-            {
-                current_processed_count_++;
-            }
+            current_processed_count_++;
             return true;
         }
         return false;
@@ -1801,10 +1797,7 @@ class PubSubReader
         eprosima::fastdds::dds::SampleInfo dds_info;
         if (datareader_->take_next_sample(data, &dds_info) == eprosima::fastdds::dds::RETCODE_OK)
         {
-            if (dds_info.publication_handle != eprosima::fastdds::dds::HANDLE_NIL)
-            {
-                current_processed_count_++;
-            }
+            current_processed_count_++;
             return true;
         }
         return false;
@@ -2024,8 +2017,7 @@ class PubSubReader
         ReturnCode_t success = take_ ?
                 datareader->take_next_sample((void*)&data, &info) :
                 datareader->read_next_sample((void*)&data, &info);
-        if ((eprosima::fastdds::dds::RETCODE_OK == success) &&
-                (info.publication_handle != eprosima::fastdds::dds::HANDLE_NIL))
+        if (eprosima::fastdds::dds::RETCODE_OK == success)
         {
             returnedValue = true;
 
@@ -2079,12 +2071,6 @@ class PubSubReader
             type& data = datas[i];
             eprosima::fastdds::dds::SampleInfo& info = infos[i];
 
-            // Skip unknown samples
-            if (info.publication_handle == eprosima::fastdds::dds::HANDLE_NIL)
-            {
-                continue;
-            }
-
             // Check order of changes.
             LastSeqInfo seq_info{ info.instance_handle, info.sample_identity.writer_guid() };
             ASSERT_LT(last_seq[seq_info], info.sample_identity.sequence_number());