diff --git a/ntcore/src/main/native/cpp/LocalStorage.cpp b/ntcore/src/main/native/cpp/LocalStorage.cpp index 3ac16531cf..b02531de64 100644 --- a/ntcore/src/main/native/cpp/LocalStorage.cpp +++ b/ntcore/src/main/native/cpp/LocalStorage.cpp @@ -77,7 +77,7 @@ struct TopicData { std::string name; Value lastValue; // also stores timestamp - bool lastValueNetwork{false}; + Value lastValueNetwork; NT_Type type{NT_UNASSIGNED}; std::string typeStr; unsigned int flags{0}; // for NT3 APIs @@ -461,7 +461,7 @@ void LSImpl::CheckReset(TopicData* topic) { return; } topic->lastValue = {}; - topic->lastValueNetwork = false; + topic->lastValueNetwork = {}; topic->type = NT_UNASSIGNED; topic->typeStr.clear(); topic->flags = 0; @@ -471,16 +471,15 @@ void LSImpl::CheckReset(TopicData* topic) { bool LSImpl::SetValue(TopicData* topic, const Value& value, unsigned int eventFlags, bool isDuplicate) { + DEBUG4("SetValue({}, {}, {}, {})", topic->name, value.time(), eventFlags, + isDuplicate); if (topic->type != NT_UNASSIGNED && topic->type != value.type()) { return false; } - bool isNetwork = (eventFlags & NT_EVENT_VALUE_REMOTE) != 0; - if (!topic->lastValue || topic->lastValueNetwork == isNetwork || - value.time() >= topic->lastValue.time()) { + if (!topic->lastValue || value.time() >= topic->lastValue.time()) { // TODO: notify option even if older value topic->type = value.type(); topic->lastValue = value; - topic->lastValueNetwork = isNetwork; NotifyValue(topic, eventFlags, isDuplicate); } if (!isDuplicate && topic->datalogType == value.type()) { @@ -1218,13 +1217,16 @@ bool LSImpl::PublishLocalValue(PublisherData* publisher, const Value& value, return false; } if (publisher->active) { - bool isDuplicate; + bool isDuplicate, isNetworkDuplicate; if (force || publisher->config.keepDuplicates) { isDuplicate = false; + isNetworkDuplicate = false; } else { isDuplicate = (publisher->topic->lastValue == value); + isNetworkDuplicate = (publisher->topic->lastValueNetwork == value); } - if (!isDuplicate && m_network) { + if (!isNetworkDuplicate && m_network) { + publisher->topic->lastValueNetwork = value; m_network->SetValue(publisher->handle, value); } return SetValue(publisher->topic, value, NT_EVENT_VALUE_LOCAL, isDuplicate); @@ -1346,8 +1348,10 @@ void LocalStorage::NetworkPropertiesUpdate(std::string_view name, void LocalStorage::NetworkSetValue(NT_Topic topicHandle, const Value& value) { std::scoped_lock lock{m_mutex}; if (auto topic = m_impl->m_topics.Get(topicHandle)) { - m_impl->SetValue(topic, value, NT_EVENT_VALUE_REMOTE, - value == topic->lastValue); + if (m_impl->SetValue(topic, value, NT_EVENT_VALUE_REMOTE, + value == topic->lastValue)) { + topic->lastValueNetwork = value; + } } } diff --git a/ntcore/src/test/native/cpp/LocalStorageTest.cpp b/ntcore/src/test/native/cpp/LocalStorageTest.cpp index 7030d55992..411d1cbabb 100644 --- a/ntcore/src/test/native/cpp/LocalStorageTest.cpp +++ b/ntcore/src/test/native/cpp/LocalStorageTest.cpp @@ -789,4 +789,22 @@ TEST_F(LocalStorageNumberVariantsTest, ReadQueue) { } } +TEST_F(LocalStorageTest, NetworkDuplicateDetect) { + EXPECT_CALL(network, Publish(_, _, _, _, _, _)); + auto pub = storage.Publish(fooTopic, NT_DOUBLE, "double", {}, {}); + auto remoteTopic = + storage.NetworkAnnounce("foo", "double", wpi::json::object(), 0); + + // local set + EXPECT_CALL(network, SetValue(_, _)); + storage.SetEntryValue(pub, Value::MakeDouble(1.0, 50)); + // 2nd local set with same value - no SetValue call to network + storage.SetEntryValue(pub, Value::MakeDouble(1.0, 60)); + // network set with different value + storage.NetworkSetValue(remoteTopic, Value::MakeDouble(2.0, 70)); + // 3rd local set with same value generates a SetValue call to network + EXPECT_CALL(network, SetValue(_, _)); + storage.SetEntryValue(pub, Value::MakeDouble(1.0, 80)); +} + } // namespace nt