eclipse-iceoryx / iceoryx

Eclipse iceoryx™ - true zero-copy inter-process-communication
https://iceoryx.io
Apache License 2.0
1.69k stars 393 forks source link

Subscriber doesn't receive the first data from publisher in icedelivery example. #62

Closed evshary closed 4 years ago

evshary commented 4 years ago

Required information

Operating system: Ubuntu 18.04 LTS

Compiler version: gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)

Observed result or behaviour: While running example program(ice-publisher-xxx and ice-subscriber-xxx), subscriber doesn't receive the first data.

Expected result or behaviour: If we run the subscriber first, and then run publisher, every data should be received by subscriber.

Conditions where it occurred / Performed steps:

budrus commented 4 years ago

Thanks @evshary . This is a nice topic. In this example we create a publisher, then we enter a loop where we publish the counter and wait for 1 sec. If you first start the subscriber it will be connected by a discovery loop in the daemon as soon as this detects the publisher. As the discovery loop has a cycle of 100 ms, you loose the first sample from the publisher that is sent immediately after creating the publisher (somehow a race). The default contract is that a subscriber will get all the samples as soon as it is subscribed. If you do not want to loose samples you need what is called history QoS in DDS, I guess. We currently already support a history of 1. This can be enabled with the publisher method enableDoDeliverOnSubscription().
So if you enable this functionality in the c'tor of the TypedPublisher https://github.com/eclipse/iceoryx/blob/master/iceoryx_examples/icedelivery/a_typed_api.hpp#L45

like this

   TypedPublisher(const iox::capro::ServiceDescription& id)
        : m_publisher(id)
    {
        m_publisher.offer();
        m_publisher.enableDoDeliverOnSubscription();
    }

it should be fine.

This "fixes" the problem in this example. Currently we only support a history of 1. But we already started a refactoring that would allow to configure a variable history size when creating publishers and subscribers

evshary commented 4 years ago

@michael-poehnl Thanks for your answer. I'll close the issue.

evshary commented 4 years ago

@michael-poehnl Sorry for reopen the issue. I face a problem while using enableDoDeliverOnSubscription(). RouDi may crash if the time interval between enableDoDeliverOnSubscription() and actual publish is large. For example, here is the modification based on ice_publisher_bare_metal.cpp

....
    myPublisher.offer();
    myPublisher.enableDoDeliverOnSubscription();
    sleep(2);  // create a large time interval

    uint32_t ct = 0;

    while (!killswitch)
    {
....

The crash log is like the following.

2020-03-26 13:57:24.595 [ Error ]: ICEORYX error! POSH__SENDERPORT_FIELD_SUBSCRIBE_WITHOUT_DATA
RouDi: /home/chenying/workspace/iceoryx_ws/src/iceoryx/iceoryx_utils/source/error_handling/error_handling.cpp:53: static void iox::ErrorHandler::ReactOnErrorLevel(iox::ErrorLevel, const char*): Assertion `false' failed.
[1]    27138 abort (core dumped)  ./RouDi

This error is related to here. https://github.com/eclipse/iceoryx/blob/master/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_handler.inl#L139 I guess the error indicates there is no data to transmit while adding new receiver and doDeliverOnSubscription(History of 1) is enable. IMHO, maybe we don't need to generate error here. History of 1 doesn't mean there must be data while new receiver subscribes.

budrus commented 4 years ago

We first supported only the AUTOSAR Adaptive API. There a history of 1 is called ‚field‘ and the contract is that it always has a value. I.e. a subscriber has to get a sample on subscription. We introduced this check to recognize if the contract is not fulfilled.

This makes no sense in a DDS / ROS2 context. We will change this, may take some days because of current circumstances. So far it should be fine if you just comment the check

elBoberido commented 4 years ago

you can also just allocate and send a sample before doing the offering

budrus commented 4 years ago

This will change in #25, so we can close this issue once we finalized #25

mossmaurice commented 4 years ago

Closing this issue, as iceoryx 1.0 release (with #25) is on the horizon.