diff --git a/ntcore/src/generate/java/Topic.java.jinja b/ntcore/src/generate/java/Topic.java.jinja index 307291df45..e22fa3baf8 100644 --- a/ntcore/src/generate/java/Topic.java.jinja +++ b/ntcore/src/generate/java/Topic.java.jinja @@ -136,6 +136,7 @@ public final class {{ TypeName }}Topic extends Topic { * @param properties JSON properties * @param options publish options * @return publisher + * @throws IllegalArgumentException if properties is not a JSON object */ public {{ TypeName }}Publisher publishEx( String typeString, diff --git a/ntcore/src/main/java/edu/wpi/first/networktables/Topic.java b/ntcore/src/main/java/edu/wpi/first/networktables/Topic.java index cc6d6b09f7..db08a344d2 100644 --- a/ntcore/src/main/java/edu/wpi/first/networktables/Topic.java +++ b/ntcore/src/main/java/edu/wpi/first/networktables/Topic.java @@ -141,6 +141,7 @@ public class Topic { * * @param name property name * @param value property value (JSON string) + * @throws IllegalArgumentException if properties is not parseable as JSON */ public void setProperty(String name, String value) { NetworkTablesJNI.setTopicProperty(m_handle, name, value); @@ -171,6 +172,7 @@ public class Topic { * values result in deletion of the corresponding property. * * @param properties JSON object string with keys to add/update/delete + * @throws IllegalArgumentException if properties is not a JSON object */ public void setProperties(String properties) { NetworkTablesJNI.setTopicProperties(m_handle, properties); @@ -242,6 +244,7 @@ public class Topic { * @param properties JSON properties * @param options publish options * @return publisher + * @throws IllegalArgumentException if properties is not a JSON object */ public GenericPublisher genericPublishEx( String typeString, String properties, PubSubOption... options) { diff --git a/ntcore/src/main/native/cpp/LocalStorage.cpp b/ntcore/src/main/native/cpp/LocalStorage.cpp index c2479fe0d7..8866ef848b 100644 --- a/ntcore/src/main/native/cpp/LocalStorage.cpp +++ b/ntcore/src/main/native/cpp/LocalStorage.cpp @@ -759,6 +759,9 @@ void LSImpl::SetRetained(TopicData* topic, bool value) { void LSImpl::SetProperties(TopicData* topic, const wpi::json& update, bool sendNetwork) { + if (!update.is_object()) { + return; + } DEBUG4("SetProperties({},{})", topic->name, sendNetwork); for (auto&& change : update.items()) { if (change.value().is_null()) { @@ -924,8 +927,13 @@ PublisherData* LSImpl::AddLocalPublisher(TopicData* topic, if (properties.is_null()) { topic->properties = wpi::json::object(); - } else { + } else if (properties.is_object()) { topic->properties = properties; + } else { + WPI_WARNING(m_logger, + "ignoring non-object properties when publishing '{}'", + topic->name); + topic->properties = wpi::json::object(); } if (topic->properties.empty()) { @@ -1812,14 +1820,17 @@ wpi::json LocalStorage::GetTopicProperties(NT_Topic topicHandle) { } } -void LocalStorage::SetTopicProperties(NT_Topic topicHandle, +bool LocalStorage::SetTopicProperties(NT_Topic topicHandle, const wpi::json& update) { if (!update.is_object()) { - return; + return false; } std::scoped_lock lock{m_mutex}; if (auto topic = m_impl->m_topics.Get(topicHandle)) { m_impl->SetProperties(topic, update, true); + return true; + } else { + return {}; } } @@ -1871,15 +1882,21 @@ NT_Publisher LocalStorage::Publish(NT_Topic topicHandle, NT_Type type, std::string_view typeStr, const wpi::json& properties, wpi::span options) { - if (type == NT_UNASSIGNED || typeStr.empty()) { - return 0; - } - std::scoped_lock lock{m_mutex}; // Get the topic auto* topic = m_impl->m_topics.Get(topicHandle); if (!topic) { + WPI_ERROR(m_impl->m_logger, "trying to publish invalid topic handle ({})", + topicHandle); + return 0; + } + + if (type == NT_UNASSIGNED || typeStr.empty()) { + WPI_ERROR( + m_impl->m_logger, + "cannot publish '{}' with an unassigned type or empty type string", + topic->name); return 0; } diff --git a/ntcore/src/main/native/cpp/LocalStorage.h b/ntcore/src/main/native/cpp/LocalStorage.h index 503783c69d..df786bcccd 100644 --- a/ntcore/src/main/native/cpp/LocalStorage.h +++ b/ntcore/src/main/native/cpp/LocalStorage.h @@ -84,7 +84,7 @@ class LocalStorage final : public net::ILocalStorage { wpi::json GetTopicProperties(NT_Topic topic); - void SetTopicProperties(NT_Topic topic, const wpi::json& update); + bool SetTopicProperties(NT_Topic topic, const wpi::json& update); TopicInfo GetTopicInfo(NT_Topic topic); diff --git a/ntcore/src/main/native/cpp/ntcore_c.cpp b/ntcore/src/main/native/cpp/ntcore_c.cpp index 0b3ad7b6d9..77854d040a 100644 --- a/ntcore/src/main/native/cpp/ntcore_c.cpp +++ b/ntcore/src/main/native/cpp/ntcore_c.cpp @@ -298,8 +298,7 @@ NT_Bool NT_SetTopicProperties(NT_Topic topic, const char* properties) { } catch (wpi::json::parse_error&) { return false; } - nt::SetTopicProperties(topic, j); - return true; + return nt::SetTopicProperties(topic, j); } NT_Subscriber NT_Subscribe(NT_Topic topic, NT_Type type, const char* typeStr, diff --git a/ntcore/src/main/native/cpp/ntcore_cpp.cpp b/ntcore/src/main/native/cpp/ntcore_cpp.cpp index f444277bb6..97c94fea5a 100644 --- a/ntcore/src/main/native/cpp/ntcore_cpp.cpp +++ b/ntcore/src/main/native/cpp/ntcore_cpp.cpp @@ -293,9 +293,11 @@ wpi::json GetTopicProperties(NT_Topic topic) { } } -void SetTopicProperties(NT_Topic topic, const wpi::json& properties) { +bool SetTopicProperties(NT_Topic topic, const wpi::json& properties) { if (auto ii = InstanceImpl::GetTyped(topic, Handle::kTopic)) { - ii->localStorage.SetTopicProperties(topic, properties); + return ii->localStorage.SetTopicProperties(topic, properties); + } else { + return {}; } } diff --git a/ntcore/src/main/native/include/networktables/Topic.h b/ntcore/src/main/native/include/networktables/Topic.h index 7f9da69c57..474a92d429 100644 --- a/ntcore/src/main/native/include/networktables/Topic.h +++ b/ntcore/src/main/native/include/networktables/Topic.h @@ -149,8 +149,9 @@ class Topic { * of the corresponding property. * * @param properties JSON object with keys to add/update/delete + * @return False if properties is not an object */ - void SetProperties(const wpi::json& properties); + bool SetProperties(const wpi::json& properties); /** * Gets combined information about the topic. @@ -340,7 +341,7 @@ class Subscriber { NT_Subscriber m_subHandle{0}; }; -/** NetworkTables pubscriber. */ +/** NetworkTables publisher. */ class Publisher { public: virtual ~Publisher(); diff --git a/ntcore/src/main/native/include/networktables/Topic.inc b/ntcore/src/main/native/include/networktables/Topic.inc index 62b65285cf..642e49eb2b 100644 --- a/ntcore/src/main/native/include/networktables/Topic.inc +++ b/ntcore/src/main/native/include/networktables/Topic.inc @@ -54,8 +54,8 @@ inline void Topic::DeleteProperty(std::string_view name) { ::nt::DeleteTopicProperty(m_handle, name); } -inline void Topic::SetProperties(const wpi::json& properties) { - ::nt::SetTopicProperties(m_handle, properties); +inline bool Topic::SetProperties(const wpi::json& properties) { + return ::nt::SetTopicProperties(m_handle, properties); } inline TopicInfo Topic::GetInfo() const { @@ -71,6 +71,9 @@ inline Subscriber::Subscriber(Subscriber&& rhs) : m_subHandle{rhs.m_subHandle} { } inline Subscriber& Subscriber::operator=(Subscriber&& rhs) { + if (m_subHandle != 0) { + ::nt::Release(m_subHandle); + } m_subHandle = rhs.m_subHandle; rhs.m_subHandle = 0; return *this; @@ -97,6 +100,9 @@ inline Publisher::Publisher(Publisher&& rhs) : m_pubHandle{rhs.m_pubHandle} { } inline Publisher& Publisher::operator=(Publisher&& rhs) { + if (m_pubHandle != 0) { + ::nt::Release(m_pubHandle); + } m_pubHandle = rhs.m_pubHandle; rhs.m_pubHandle = 0; return *this; diff --git a/ntcore/src/main/native/include/ntcore_c.h b/ntcore/src/main/native/include/ntcore_c.h index a2deb83b58..ee9113b74e 100644 --- a/ntcore/src/main/native/include/ntcore_c.h +++ b/ntcore/src/main/native/include/ntcore_c.h @@ -671,6 +671,7 @@ char* NT_GetTopicProperties(NT_Topic topic, size_t* len); * * @param topic topic handle * @param properties JSON object string with keys to add/update/delete + * @return False if properties are not a valid JSON object */ NT_Bool NT_SetTopicProperties(NT_Topic topic, const char* properties); diff --git a/ntcore/src/main/native/include/ntcore_cpp.h b/ntcore/src/main/native/include/ntcore_cpp.h index 35547d5154..74f6ff58a2 100644 --- a/ntcore/src/main/native/include/ntcore_cpp.h +++ b/ntcore/src/main/native/include/ntcore_cpp.h @@ -651,8 +651,9 @@ wpi::json GetTopicProperties(NT_Topic topic); * * @param topic topic handle * @param update JSON object with keys to add/update/delete + * @return False if update is not a JSON object */ -void SetTopicProperties(NT_Topic topic, const wpi::json& update); +bool SetTopicProperties(NT_Topic topic, const wpi::json& update); /** * Creates a new subscriber to value changes on a topic.