diff --git a/include/networktables/NetworkTable.h b/include/networktables/NetworkTable.h index d3515ad5a4..70f164f1d3 100644 --- a/include/networktables/NetworkTable.h +++ b/include/networktables/NetworkTable.h @@ -137,12 +137,11 @@ class NetworkTable : public ITable { void AddTableListener(ITableListener* listener); void AddTableListener(ITableListener* listener, bool immediateNotify); - void AddTableListener(ITableListener* listener, bool immediateNotify, - bool localNotify); + void AddTableListenerEx(ITableListener* listener, unsigned int flags); void AddTableListener(llvm::StringRef key, ITableListener* listener, bool immediateNotify); - void AddTableListener(llvm::StringRef key, ITableListener* listener, - bool immediateNotify, bool localNotify); + void AddTableListenerEx(llvm::StringRef key, ITableListener* listener, + unsigned int flags); void AddSubTableListener(ITableListener* listener); void AddSubTableListener(ITableListener* listener, bool localNotify); void RemoveTableListener(ITableListener* listener); diff --git a/include/ntcore_c.h b/include/ntcore_c.h index eecff87ccf..07e4c087e7 100644 --- a/include/ntcore_c.h +++ b/include/ntcore_c.h @@ -48,6 +48,17 @@ enum NT_LogLevel { NT_LOG_DEBUG4 = 6 }; +/** NetworkTables notififier kinds. */ +enum NT_NotifyKind { + NT_NOTIFY_NONE = 0, + NT_NOTIFY_IMMEDIATE = 0x01, /* initial listener addition */ + NT_NOTIFY_LOCAL = 0x02, /* changed locally */ + NT_NOTIFY_NEW = 0x04, /* newly created entry */ + NT_NOTIFY_DELETE = 0x08, /* deleted */ + NT_NOTIFY_UPDATE = 0x10, /* value changed */ + NT_NOTIFY_FLAGS = 0x20 /* flags changed */ +}; + /* * Structures */ @@ -258,7 +269,7 @@ void NT_Flush(void); typedef void (*NT_EntryListenerCallback)( unsigned int uid, void *data, const char *name, size_t name_len, - const struct NT_Value *value, int is_new); + const struct NT_Value *value, unsigned int flags); typedef void (*NT_ConnectionListenerCallback)( unsigned int uid, void *data, int connected, @@ -266,7 +277,7 @@ typedef void (*NT_ConnectionListenerCallback)( unsigned int NT_AddEntryListener(const char *prefix, size_t prefix_len, void *data, NT_EntryListenerCallback callback, - int immediate_notify, int local_notify); + unsigned int flags); void NT_RemoveEntryListener(unsigned int entry_listener_uid); unsigned int NT_AddConnectionListener(void *data, NT_ConnectionListenerCallback callback, diff --git a/include/ntcore_cpp.h b/include/ntcore_cpp.h index 302150bdac..ae9ae4c3ee 100644 --- a/include/ntcore_cpp.h +++ b/include/ntcore_cpp.h @@ -181,15 +181,15 @@ void Flush(); */ typedef std::function value, bool is_new)> - EntryListenerCallback; + std::shared_ptr value, + unsigned int flags)> EntryListenerCallback; typedef std::function ConnectionListenerCallback; unsigned int AddEntryListener(StringRef prefix, EntryListenerCallback callback, - bool immediate_notify, bool local_notify); + unsigned int flags); void RemoveEntryListener(unsigned int entry_listener_uid); unsigned int AddConnectionListener(ConnectionListenerCallback callback, bool immediate_notify); diff --git a/include/tables/ITable.h b/include/tables/ITable.h index a819b6affe..9b93da9860 100644 --- a/include/tables/ITable.h +++ b/include/tables/ITable.h @@ -215,11 +215,10 @@ class ITable { * @param listener the listener to add * @param immediateNotify if true then this listener will be notified of all * current entries (marked as new) - * @param localNotify if true then this listener will be notified of all - * local changes in addition to all remote changes + * @param flags bitmask of NT_NotifyKind specifying desired notifications */ - virtual void AddTableListener(ITableListener* listener, bool immediateNotify, - bool localNotify) = 0; + virtual void AddTableListenerEx(ITableListener* listener, + unsigned int flags) = 0; /** * Add a listener for changes to a specific key the table @@ -239,11 +238,10 @@ class ITable { * @param listener the listener to add * @param immediateNotify if true then this listener will be notified of all * current entries (marked as new) - * @param localNotify if true then this listener will be notified of all - * local changes in addition to all remote changes + * @param flags bitmask of NT_NotifyKind specifying desired notifications */ - virtual void AddTableListener(llvm::StringRef key, ITableListener* listener, - bool immediateNotify, bool localNotify) = 0; + virtual void AddTableListenerEx(llvm::StringRef key, ITableListener* listener, + unsigned int flags) = 0; /** * This will immediately notify the listener of all current sub tables diff --git a/include/tables/ITableListener.h b/include/tables/ITableListener.h index 21b8c2ce7a..6b728d28a7 100644 --- a/include/tables/ITableListener.h +++ b/include/tables/ITableListener.h @@ -20,8 +20,6 @@ class ITableListener { virtual ~ITableListener() = default; /** * Called when a key-value pair is changed in a {@link ITable} - * WARNING: If a new key-value is put in this method value changed will - * immediatly be called which could lead to recursive code * @param source the table the key-value pair exists in * @param key the key associated with the value that changed * @param value the new value @@ -30,7 +28,23 @@ class ITableListener { */ virtual void ValueChanged(ITable* source, llvm::StringRef key, - std::shared_ptr value, bool isNew) = 0; + std::shared_ptr value, + bool isNew) = 0; + + /** + * Extended version of ValueChanged. Called when a key-value pair is + * changed in a {@link ITable}. The default implementation simply calls + * ValueChanged(). If this is overridden, ValueChanged() will not be called. + * @param source the table the key-value pair exists in + * @param key the key associated with the value that changed + * @param value the new value + * @param flags update flags; for example, NT_NOTIFY_NEW if the key did not + * previously exist in the table + */ + virtual void ValueChangedEx(ITable* source, + llvm::StringRef key, + std::shared_ptr value, + unsigned int flags); }; #endif /* ITABLELISTENER_H_ */ diff --git a/java/lib/NetworkTablesJNI.cpp b/java/lib/NetworkTablesJNI.cpp index 5fa652ed7a..af205d0b71 100644 --- a/java/lib/NetworkTablesJNI.cpp +++ b/java/lib/NetworkTablesJNI.cpp @@ -887,8 +887,7 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_networktables_NetworkTablesJNI * Signature: (Ljava/lang/String;Ledu/wpi/first/wpilibj/networktables/NetworkTablesJNI/EntryListenerFunction;Z)I */ JNIEXPORT jint JNICALL Java_edu_wpi_first_wpilibj_networktables_NetworkTablesJNI_addEntryListener - (JNIEnv *envouter, jclass, jstring prefix, jobject listener, - jboolean immediateNotify, jboolean localNotify) + (JNIEnv *envouter, jclass, jstring prefix, jobject listener, jint flags) { // the shared pointer to the weak global will keep it around until the // entry listener is destroyed @@ -901,13 +900,13 @@ JNIEXPORT jint JNICALL Java_edu_wpi_first_wpilibj_networktables_NetworkTablesJNI // method ids, on the other hand, are safe to retain jmethodID mid = envouter->GetMethodID( - cls, "apply", "(ILjava/lang/String;Ljava/lang/Object;Z)V"); + cls, "apply", "(ILjava/lang/String;Ljava/lang/Object;I)V"); if (!mid) return 0; return nt::AddEntryListener( JavaStringRef(envouter, prefix), [=](unsigned int uid, nt::StringRef name, - std::shared_ptr value, bool is_new) { + std::shared_ptr value, unsigned int flags_) { // need to attach as we're coming from a separate thread here if (!jvm) return; JNIEnv *env; @@ -929,14 +928,13 @@ JNIEXPORT jint JNICALL Java_edu_wpi_first_wpilibj_networktables_NetworkTablesJNI goto done; } env->CallVoidMethod(handler, mid, (jint)uid, ToJavaString(env, name), - jobj, (jboolean)(is_new ? 1 : 0)); + jobj, (jint)(flags_)); if (env->ExceptionCheck()) env->ExceptionDescribe(); } done: jvm->DetachCurrentThread(); }, - immediateNotify != JNI_FALSE, - localNotify != JNI_FALSE); + flags); } /* diff --git a/java/src/edu/wpi/first/wpilibj/networktables/NetworkTable.java b/java/src/edu/wpi/first/wpilibj/networktables/NetworkTable.java index 4bb4113641..ea0844aa12 100644 --- a/java/src/edu/wpi/first/wpilibj/networktables/NetworkTable.java +++ b/java/src/edu/wpi/first/wpilibj/networktables/NetworkTable.java @@ -240,12 +240,15 @@ public class NetworkTable implements ITable, IRemote { } public void addTableListener(ITableListener listener) { - addTableListener(listener, false, false); + addTableListenerEx(listener, NOTIFY_NEW | NOTIFY_UPDATE); } public void addTableListener(ITableListener listener, boolean immediateNotify) { - addTableListener(listener, immediateNotify, false); + int flags = NOTIFY_NEW | NOTIFY_UPDATE; + if (immediateNotify) + flags |= NOTIFY_IMMEDIATE; + addTableListenerEx(listener, flags); } private class TableListenerAdapter extends ListenerBase implements NetworkTablesJNI.EntryListenerFunction { @@ -259,18 +262,17 @@ public class NetworkTable implements ITable, IRemote { this.targetListener = targetListener; } - public void apply(int uid, String key, Object value, boolean isNew) { + public void apply(int uid, String key, Object value, int flags) { String relativeKey = key.substring(prefixLen); if (relativeKey.indexOf(PATH_SEPARATOR) != -1) return; - targetListener.valueChanged(targetSource, relativeKey, value, isNew); + targetListener.valueChangedEx(targetSource, relativeKey, value, flags); } } private final Hashtable> listenerMap = new Hashtable>(); - public synchronized void addTableListener(ITableListener listener, - boolean immediateNotify, - boolean localNotify) { + public synchronized void addTableListenerEx(ITableListener listener, + int flags) { List adapters = listenerMap.get(listener); if (adapters == null) { adapters = new ArrayList(); @@ -278,13 +280,16 @@ public class NetworkTable implements ITable, IRemote { } TableListenerAdapter adapter = new TableListenerAdapter(path.length() + 1, this, listener); - adapter.uid = NetworkTablesJNI.addEntryListener(path + PATH_SEPARATOR, adapter, immediateNotify, localNotify); + adapter.uid = NetworkTablesJNI.addEntryListener(path + PATH_SEPARATOR, adapter, flags); adapters.add(adapter); } public void addTableListener(String key, ITableListener listener, boolean immediateNotify) { - addTableListener(key, listener, immediateNotify, false); + int flags = NOTIFY_NEW | NOTIFY_UPDATE; + if (immediateNotify) + flags |= NOTIFY_IMMEDIATE; + addTableListenerEx(key, listener, flags); } private class KeyListenerAdapter extends ListenerBase implements NetworkTablesJNI.EntryListenerFunction { @@ -300,16 +305,16 @@ public class NetworkTable implements ITable, IRemote { this.targetListener = targetListener; } - public void apply(int uid, String key, Object value, boolean isNew) { + public void apply(int uid, String key, Object value, int flags) { if (!key.equals(fullKey)) return; - targetListener.valueChanged(targetSource, relativeKey, value, isNew); + targetListener.valueChangedEx(targetSource, relativeKey, value, flags); } } - public synchronized void addTableListener(String key, ITableListener listener, - boolean immediateNotify, - boolean localNotify) { + public synchronized void addTableListenerEx(String key, + ITableListener listener, + int flags) { List adapters = listenerMap.get(listener); if (adapters == null) { adapters = new ArrayList(); @@ -318,7 +323,7 @@ public class NetworkTable implements ITable, IRemote { String fullKey = path + PATH_SEPARATOR + key; KeyListenerAdapter adapter = new KeyListenerAdapter(key, fullKey, this, listener); - adapter.uid = NetworkTablesJNI.addEntryListener(fullKey, adapter, immediateNotify, localNotify); + adapter.uid = NetworkTablesJNI.addEntryListener(fullKey, adapter, flags); adapters.add(adapter); } @@ -338,7 +343,7 @@ public class NetworkTable implements ITable, IRemote { this.targetListener = targetListener; } - public void apply(int uid, String key, Object value, boolean isNew) { + public void apply(int uid, String key, Object value, int flags) { String relativeKey = key.substring(prefixLen); int endSubTable = relativeKey.indexOf(PATH_SEPARATOR); if (endSubTable == -1) @@ -347,7 +352,7 @@ public class NetworkTable implements ITable, IRemote { if (notifiedTables.contains(subTableKey)) return; notifiedTables.add(subTableKey); - targetListener.valueChanged(targetSource, subTableKey, targetSource.getSubTable(subTableKey), true); + targetListener.valueChangedEx(targetSource, subTableKey, targetSource.getSubTable(subTableKey), flags); } } @@ -360,7 +365,10 @@ public class NetworkTable implements ITable, IRemote { } SubListenerAdapter adapter = new SubListenerAdapter(path.length() + 1, this, listener); - adapter.uid = NetworkTablesJNI.addEntryListener(path + PATH_SEPARATOR, adapter, true, localNotify); + int flags = NOTIFY_NEW | NOTIFY_IMMEDIATE; + if (localNotify) + flags |= NOTIFY_LOCAL; + adapter.uid = NetworkTablesJNI.addEntryListener(path + PATH_SEPARATOR, adapter, flags); adapters.add(adapter); } diff --git a/java/src/edu/wpi/first/wpilibj/networktables/NetworkTablesJNI.java b/java/src/edu/wpi/first/wpilibj/networktables/NetworkTablesJNI.java index d7064f56f4..f6e02219ed 100644 --- a/java/src/edu/wpi/first/wpilibj/networktables/NetworkTablesJNI.java +++ b/java/src/edu/wpi/first/wpilibj/networktables/NetworkTablesJNI.java @@ -107,9 +107,9 @@ public class NetworkTablesJNI { public static native void flush(); public interface EntryListenerFunction { - void apply(int uid, String key, Object value, boolean isNew); + void apply(int uid, String key, Object value, int flags); } - public static native int addEntryListener(String prefix, EntryListenerFunction listener, boolean immediateNotify, boolean localNotify); + public static native int addEntryListener(String prefix, EntryListenerFunction listener, int flags); public static native void removeEntryListener(int entryListenerUid); public interface ConnectionListenerFunction { diff --git a/java/src/edu/wpi/first/wpilibj/tables/ITable.java b/java/src/edu/wpi/first/wpilibj/tables/ITable.java index 2c2534f3d1..40ad37dd73 100644 --- a/java/src/edu/wpi/first/wpilibj/tables/ITable.java +++ b/java/src/edu/wpi/first/wpilibj/tables/ITable.java @@ -297,6 +297,14 @@ public interface ITable { */ public String[] getStringArray(String key, String[] defaultValue); + /** Notifier flag values. */ + public static final int NOTIFY_IMMEDIATE = 0x01; + public static final int NOTIFY_LOCAL = 0x02; + public static final int NOTIFY_NEW = 0x04; + public static final int NOTIFY_DELETE = 0x08; + public static final int NOTIFY_UPDATE = 0x10; + public static final int NOTIFY_FLAGS = 0x20; + /** * Add a listener for changes to the table * @param listener the listener to add @@ -313,13 +321,9 @@ public interface ITable { /** * Add a listener for changes to the table * @param listener the listener to add - * @param immediateNotify if true then this listener will be notified of all - * current entries (marked as new) - * @param localNotify if true then this listener will be notified of all - * local changes in addition to all remote changes + * @param flags bitmask specifying desired notifications */ - public void addTableListener(ITableListener listener, boolean immediateNotify, - boolean localNotify); + public void addTableListenerEx(ITableListener listener, int flags); /** * Add a listener for changes to a specific key the table @@ -334,13 +338,10 @@ public interface ITable { * Add a listener for changes to a specific key the table * @param key the key to listen for * @param listener the listener to add - * @param immediateNotify if true then this listener will be notified of all - * current entries (marked as new) - * @param localNotify if true then this listener will be notified of all - * local changes in addition to all remote changes + * @param flags bitmask specifying desired notifications */ - public void addTableListener(String key, ITableListener listener, - boolean immediateNotify, boolean localNotify); + public void addTableListenerEx(String key, ITableListener listener, + int flags); /** * This will immediately notify the listener of all current sub tables * @param listener diff --git a/java/src/edu/wpi/first/wpilibj/tables/ITableListener.java b/java/src/edu/wpi/first/wpilibj/tables/ITableListener.java index f53528f180..30c0115b2c 100644 --- a/java/src/edu/wpi/first/wpilibj/tables/ITableListener.java +++ b/java/src/edu/wpi/first/wpilibj/tables/ITableListener.java @@ -9,11 +9,26 @@ package edu.wpi.first.wpilibj.tables; public interface ITableListener { /** * Called when a key-value pair is changed in a {@link ITable} - * WARNING: If a new key-value is put in this method value changed will immediatly be called which could lead to recursive code * @param source the table the key-value pair exists in * @param key the key associated with the value that changed * @param value the new value * @param isNew true if the key did not previously exist in the table, otherwise it is false */ public void valueChanged(ITable source, String key, Object value, boolean isNew); + + /** + * Extended version of valueChanged. Called when a key-value pair is + * changed in a {@link ITable}. The default implementation simply calls + * valueChanged(). If this is overridden, valueChanged() will not be + * called. + * @param source the table the key-value pair exists in + * @param key the key associated with the value that changed + * @param value the new value + * @param flags update flags; for example, NOTIFY_NEW if the key did not + * previously exist in the table + */ + default public void valueChangedEx(ITable source, String key, Object value, int flags) { + // NOTIFY_NEW = 0x04 + valueChanged(source, key, value, (flags & 0x04) != 0); + } } diff --git a/src/Notifier.cpp b/src/Notifier.cpp index 634c8e2b52..34d311c067 100644 --- a/src/Notifier.cpp +++ b/src/Notifier.cpp @@ -58,7 +58,7 @@ void Notifier::ThreadMain() { if (item.only) { // Don't hold mutex during callback execution! lock.unlock(); - item.only(0, name, item.value, item.is_new); + item.only(0, name, item.value, item.flags); lock.lock(); continue; } @@ -66,12 +66,29 @@ void Notifier::ThreadMain() { // Use index because iterator might get invalidated. for (std::size_t i=0; i lock(m_mutex); unsigned int uid = m_entry_listeners.size(); - m_entry_listeners.emplace_back(prefix, callback, local_notify); - if (local_notify) m_local_notifiers = true; + m_entry_listeners.emplace_back(prefix, callback, flags); + if ((flags & NT_NOTIFY_LOCAL) != 0) m_local_notifiers = true; return uid + 1; } @@ -120,12 +137,13 @@ void Notifier::RemoveEntryListener(unsigned int entry_listener_uid) { } void Notifier::NotifyEntry(StringRef name, std::shared_ptr value, - bool is_new, bool is_local, - EntryListenerCallback only) { + unsigned int flags, EntryListenerCallback only) { if (!m_active) return; - if (is_local && !m_local_notifiers) return; // optimization + // optimization: don't generate needless local queue entries if we have + // no local listeners (as this is a common case on the server side) + if ((flags & NT_NOTIFY_LOCAL) != 0 && !m_local_notifiers) return; std::unique_lock lock(m_mutex); - m_entry_notifications.emplace(name, value, is_new, is_local, only); + m_entry_notifications.emplace(name, value, flags, only); lock.unlock(); m_cond.notify_one(); } diff --git a/src/Notifier.h b/src/Notifier.h index 6a8f7bfca3..952259752e 100644 --- a/src/Notifier.h +++ b/src/Notifier.h @@ -34,15 +34,16 @@ class Notifier { void Stop(); bool active() const { return m_active; } + bool local_notifiers() const { return m_local_notifiers; } static bool destroyed() { return s_destroyed; } unsigned int AddEntryListener(StringRef prefix, EntryListenerCallback callback, - bool local_notify); + unsigned int flags); void RemoveEntryListener(unsigned int entry_listener_uid); - void NotifyEntry(StringRef name, std::shared_ptr value, bool is_new, - bool is_local, EntryListenerCallback only = nullptr); + void NotifyEntry(StringRef name, std::shared_ptr value, + unsigned int flags, EntryListenerCallback only = nullptr); unsigned int AddConnectionListener(ConnectionListenerCallback callback); void RemoveConnectionListener(unsigned int conn_listener_uid); @@ -63,29 +64,27 @@ class Notifier { struct EntryListener { EntryListener(StringRef prefix_, EntryListenerCallback callback_, - bool local_notify_) - : prefix(prefix_), callback(callback_), local_notify(local_notify_) {} + unsigned int flags_) + : prefix(prefix_), callback(callback_), flags(flags_) {} std::string prefix; EntryListenerCallback callback; - bool local_notify; + unsigned int flags; }; std::vector m_entry_listeners; std::vector m_conn_listeners; struct EntryNotification { EntryNotification(StringRef name_, std::shared_ptr value_, - bool is_new_, bool is_local_, EntryListenerCallback only_) + unsigned int flags_, EntryListenerCallback only_) : name(name_), value(value_), - is_new(is_new_), - is_local(is_local_), + flags(flags_), only(only_) {} std::string name; std::shared_ptr value; - bool is_new; - bool is_local; + unsigned int flags; EntryListenerCallback only; }; std::queue m_entry_notifications; diff --git a/src/Storage.cpp b/src/Storage.cpp index d113b9aeec..4a69bb1ade 100644 --- a/src/Storage.cpp +++ b/src/Storage.cpp @@ -93,7 +93,7 @@ void Storage::ProcessIncoming(std::shared_ptr msg, if (entry->IsPersistent()) m_persistent_dirty = true; // notify - m_notifier.NotifyEntry(name, entry->value, true, false); + m_notifier.NotifyEntry(name, entry->value, NT_NOTIFY_NEW); // send the assignment to everyone (including the originator) if (m_queue_outgoing) { @@ -135,7 +135,7 @@ void Storage::ProcessIncoming(std::shared_ptr msg, m_idmap[id] = new_entry.get(); // notify - m_notifier.NotifyEntry(name, new_entry->value, true, false); + m_notifier.NotifyEntry(name, new_entry->value, NT_NOTIFY_NEW); return; } may_need_update = true; // we may need to send an update message @@ -177,12 +177,16 @@ void Storage::ProcessIncoming(std::shared_ptr msg, return; } + unsigned int notify_flags = NT_NOTIFY_UPDATE; + // don't update flags from a <3.0 remote (not part of message) // don't update flags if this is a server response to a client id request if (!may_need_update && conn->proto_rev() >= 0x0300) { // update persistent dirty flag if persistent flag changed if ((entry->flags & NT_PERSISTENT) != (msg->flags() & NT_PERSISTENT)) m_persistent_dirty = true; + if (entry->flags != msg->flags()) + notify_flags |= NT_NOTIFY_FLAGS; entry->flags = msg->flags(); } @@ -195,7 +199,7 @@ void Storage::ProcessIncoming(std::shared_ptr msg, entry->seq_num = seq_num; // notify - m_notifier.NotifyEntry(name, entry->value, false, false); + m_notifier.NotifyEntry(name, entry->value, notify_flags); // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) @@ -232,7 +236,7 @@ void Storage::ProcessIncoming(std::shared_ptr msg, if (entry->IsPersistent()) m_persistent_dirty = true; // notify - m_notifier.NotifyEntry(entry->name, entry->value, false, false); + m_notifier.NotifyEntry(entry->name, entry->value, NT_NOTIFY_UPDATE); // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) @@ -254,6 +258,9 @@ void Storage::ProcessIncoming(std::shared_ptr msg, } Entry* entry = m_idmap[id]; + // ignore if flags didn't actually change + if (entry->flags == msg->flags()) return; + // update persistent dirty flag if persistent flag changed if ((entry->flags & NT_PERSISTENT) != (msg->flags() & NT_PERSISTENT)) m_persistent_dirty = true; @@ -261,6 +268,9 @@ void Storage::ProcessIncoming(std::shared_ptr msg, // update local entry->flags = msg->flags(); + // notify + m_notifier.NotifyEntry(entry->name, entry->value, NT_NOTIFY_FLAGS); + // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) if (m_server && m_queue_outgoing) { @@ -284,9 +294,19 @@ void Storage::ProcessIncoming(std::shared_ptr msg, // update persistent dirty flag if it's a persistent value if (entry->IsPersistent()) m_persistent_dirty = true; - // update local - m_entries.erase(entry->name); // erase from map - m_idmap[id] = nullptr; // delete it from idmap too + // delete it from idmap + m_idmap[id] = nullptr; + + // get entry (as we'll need it for notify) and erase it from the map + // it should always be in the map, but sanity check just in case + auto i = m_entries.find(entry->name); + if (i != m_entries.end()) { + auto entry2 = std::move(i->getValue()); // move the value out + m_entries.erase(i); + + // notify + m_notifier.NotifyEntry(entry2->name, entry2->value, NT_NOTIFY_DELETE); + } // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) @@ -299,12 +319,18 @@ void Storage::ProcessIncoming(std::shared_ptr msg, } case Message::kClearEntries: { // update local - m_entries.clear(); + EntriesMap map; + m_entries.swap(map); m_idmap.resize(0); // set persistent dirty flag m_persistent_dirty = true; + // notify + for (auto& entry : map) + m_notifier.NotifyEntry(entry.getKey(), entry.getValue()->value, + NT_NOTIFY_DELETE); + // broadcast to all other connections (note for client there won't // be any other connections, so don't bother) if (m_server && m_queue_outgoing) { @@ -401,7 +427,7 @@ void Storage::ApplyInitialAssignments( entry->flags = msg->flags(); entry->seq_num = seq_num; // notify - m_notifier.NotifyEntry(name, entry->value, true, false); + m_notifier.NotifyEntry(name, entry->value, NT_NOTIFY_NEW); } else { // if reconnect and sequence number not higher than local, then we // don't update the local value and instead send it back to the server @@ -412,10 +438,14 @@ void Storage::ApplyInitialAssignments( } else { entry->value = msg->value(); entry->seq_num = seq_num; + unsigned int notify_flags = NT_NOTIFY_UPDATE; // don't update flags from a <3.0 remote (not part of message) - if (conn.proto_rev() >= 0x0300) entry->flags = msg->flags(); + if (conn.proto_rev() >= 0x0300) { + if (entry->flags != msg->flags()) notify_flags |= NT_NOTIFY_FLAGS; + entry->flags = msg->flags(); + } // notify - m_notifier.NotifyEntry(name, entry->value, false, false); + m_notifier.NotifyEntry(name, entry->value, notify_flags); } } @@ -474,7 +504,7 @@ bool Storage::SetEntryValue(StringRef name, std::shared_ptr value) { value, entry->flags); lock.unlock(); queue_outgoing(msg, nullptr, nullptr); - m_notifier.NotifyEntry(name, value, true, true); + m_notifier.NotifyEntry(name, value, NT_NOTIFY_NEW | NT_NOTIFY_LOCAL); } else if (*old_value != *value) { ++entry->seq_num; // don't send an update if we don't have an assigned id yet @@ -484,7 +514,7 @@ bool Storage::SetEntryValue(StringRef name, std::shared_ptr value) { lock.unlock(); queue_outgoing(msg, nullptr, nullptr); } - m_notifier.NotifyEntry(name, value, false, true); + m_notifier.NotifyEntry(name, value, NT_NOTIFY_UPDATE | NT_NOTIFY_LOCAL); } return true; } @@ -519,7 +549,7 @@ void Storage::SetEntryTypeValue(StringRef name, std::shared_ptr value) { value, entry->flags); lock.unlock(); queue_outgoing(msg, nullptr, nullptr); - m_notifier.NotifyEntry(name, value, true, true); + m_notifier.NotifyEntry(name, value, NT_NOTIFY_NEW | NT_NOTIFY_LOCAL); } else { ++entry->seq_num; // don't send an update if we don't have an assigned id yet @@ -529,7 +559,7 @@ void Storage::SetEntryTypeValue(StringRef name, std::shared_ptr value) { lock.unlock(); queue_outgoing(msg, nullptr, nullptr); } - m_notifier.NotifyEntry(name, value, false, true); + m_notifier.NotifyEntry(name, value, NT_NOTIFY_UPDATE | NT_NOTIFY_LOCAL); } } @@ -556,6 +586,9 @@ void Storage::SetEntryFlags(StringRef name, unsigned int flags) { lock.unlock(); queue_outgoing(Message::FlagsUpdate(id, flags), nullptr, nullptr); } + + // notify + m_notifier.NotifyEntry(name, entry->value, NT_NOTIFY_FLAGS | NT_NOTIFY_LOCAL); } unsigned int Storage::GetEntryFlags(StringRef name) const { @@ -568,9 +601,8 @@ void Storage::DeleteEntry(StringRef name) { std::unique_lock lock(m_mutex); auto i = m_entries.find(name); if (i == m_entries.end()) return; - Entry* entry = i->getValue().get(); + auto entry = std::move(i->getValue()); unsigned int id = entry->id; - bool had_value = entry->value != nullptr; // update persistent dirty flag if it's a persistent value if (entry->IsPersistent()) m_persistent_dirty = true; @@ -578,7 +610,7 @@ void Storage::DeleteEntry(StringRef name) { m_entries.erase(i); // erase from map if (id < m_idmap.size()) m_idmap[id] = nullptr; - if (!had_value) return; + if (!entry->value) return; // if it had a value, generate message // don't send an update if we don't have an assigned id yet @@ -588,12 +620,17 @@ void Storage::DeleteEntry(StringRef name) { lock.unlock(); queue_outgoing(Message::EntryDelete(id), nullptr, nullptr); } + + // notify + m_notifier.NotifyEntry(name, entry->value, + NT_NOTIFY_DELETE | NT_NOTIFY_LOCAL); } void Storage::DeleteAllEntries() { std::unique_lock lock(m_mutex); if (m_entries.empty()) return; - m_entries.clear(); + EntriesMap map; + m_entries.swap(map); m_idmap.resize(0); // set persistent dirty flag @@ -604,6 +641,13 @@ void Storage::DeleteAllEntries() { auto queue_outgoing = m_queue_outgoing; lock.unlock(); queue_outgoing(Message::ClearEntries(), nullptr, nullptr); + + // notify + if (m_notifier.local_notifiers()) { + for (auto& entry : map) + m_notifier.NotifyEntry(entry.getKey(), entry.getValue()->value, + NT_NOTIFY_DELETE | NT_NOTIFY_LOCAL); + } } std::vector Storage::GetEntryInfo(StringRef prefix, @@ -631,7 +675,8 @@ void Storage::NotifyEntries(StringRef prefix, std::lock_guard lock(m_mutex); for (auto& i : m_entries) { if (!i.getKey().startswith(prefix)) continue; - m_notifier.NotifyEntry(i.getKey(), i.getValue()->value, false, false, only); + m_notifier.NotifyEntry(i.getKey(), i.getValue()->value, NT_NOTIFY_IMMEDIATE, + only); } } diff --git a/src/networktables/NetworkTable.cpp b/src/networktables/NetworkTable.cpp index 07b6e982c9..f452992446 100644 --- a/src/networktables/NetworkTable.cpp +++ b/src/networktables/NetworkTable.cpp @@ -97,16 +97,18 @@ NetworkTable::~NetworkTable() { } void NetworkTable::AddTableListener(ITableListener* listener) { - AddTableListener(listener, false, false); + AddTableListenerEx(listener, NT_NOTIFY_NEW | NT_NOTIFY_UPDATE); } void NetworkTable::AddTableListener(ITableListener* listener, bool immediateNotify) { - AddTableListener(listener, immediateNotify, false); + unsigned int flags = NT_NOTIFY_NEW | NT_NOTIFY_UPDATE; + if (immediateNotify) flags |= NT_NOTIFY_IMMEDIATE; + AddTableListenerEx(listener, flags); } -void NetworkTable::AddTableListener(ITableListener* listener, - bool immediateNotify, bool localNotify) { +void NetworkTable::AddTableListenerEx(ITableListener* listener, + unsigned int flags) { std::lock_guard lock(m_mutex); llvm::SmallString<128> path(m_path); path += PATH_SEPARATOR_CHAR; @@ -114,23 +116,24 @@ void NetworkTable::AddTableListener(ITableListener* listener, unsigned int id = nt::AddEntryListener( path, [=](unsigned int uid, StringRef name, std::shared_ptr value, - bool is_new) { + unsigned int flags_) { StringRef relative_key = name.substr(prefix_len); if (relative_key.find(PATH_SEPARATOR_CHAR) != StringRef::npos) return; - listener->ValueChanged(this, relative_key, value, is_new); + listener->ValueChangedEx(this, relative_key, value, flags_); }, - immediateNotify, - localNotify); + flags); m_listeners.emplace_back(listener, id); } void NetworkTable::AddTableListener(StringRef key, ITableListener* listener, bool immediateNotify) { - AddTableListener(key, listener, immediateNotify, false); + unsigned int flags = NT_NOTIFY_NEW | NT_NOTIFY_UPDATE; + if (immediateNotify) flags |= NT_NOTIFY_IMMEDIATE; + AddTableListenerEx(key, listener, flags); } -void NetworkTable::AddTableListener(StringRef key, ITableListener* listener, - bool immediateNotify, bool localNotify) { +void NetworkTable::AddTableListenerEx(StringRef key, ITableListener* listener, + unsigned int flags) { std::lock_guard lock(m_mutex); llvm::SmallString<128> path(m_path); path += PATH_SEPARATOR_CHAR; @@ -139,12 +142,11 @@ void NetworkTable::AddTableListener(StringRef key, ITableListener* listener, unsigned int id = nt::AddEntryListener( path, [=](unsigned int uid, StringRef name, std::shared_ptr value, - bool is_new) { + unsigned int flags_) { if (name != path) return; - listener->ValueChanged(this, name.substr(prefix_len), value, is_new); + listener->ValueChangedEx(this, name.substr(prefix_len), value, flags_); }, - immediateNotify, - localNotify); + flags); m_listeners.emplace_back(listener, id); } @@ -163,10 +165,12 @@ void NetworkTable::AddSubTableListener(ITableListener* listener, // a shared_ptr to it. auto notified_tables = std::make_shared>(); + unsigned int flags = NT_NOTIFY_NEW | NT_NOTIFY_IMMEDIATE; + if (localNotify) flags |= NT_NOTIFY_LOCAL; unsigned int id = nt::AddEntryListener( path, [=](unsigned int uid, StringRef name, std::shared_ptr value, - bool is_new) mutable { + unsigned int flags_) mutable { StringRef relative_key = name.substr(prefix_len); auto end_sub_table = relative_key.find(PATH_SEPARATOR_CHAR); if (end_sub_table == StringRef::npos) return; @@ -174,10 +178,9 @@ void NetworkTable::AddSubTableListener(ITableListener* listener, if (notified_tables->find(sub_table_key) == notified_tables->end()) return; notified_tables->insert(std::make_pair(sub_table_key, '\0')); - listener->ValueChanged(this, sub_table_key, nullptr, true); + listener->ValueChangedEx(this, sub_table_key, nullptr, flags_); }, - true, - localNotify); + flags); m_listeners.emplace_back(listener, id); } diff --git a/src/ntcore_c.cpp b/src/ntcore_c.cpp index e519a2ed85..c605944e6b 100644 --- a/src/ntcore_c.cpp +++ b/src/ntcore_c.cpp @@ -169,15 +169,14 @@ void NT_Flush(void) { nt::Flush(); } unsigned int NT_AddEntryListener(const char *prefix, size_t prefix_len, void *data, NT_EntryListenerCallback callback, - int immediate_notify, int local_notify) { + unsigned int flags) { return nt::AddEntryListener( StringRef(prefix, prefix_len), [=](unsigned int uid, StringRef name, std::shared_ptr value, - bool is_new) { - callback(uid, data, name.data(), name.size(), &value->value(), is_new); + unsigned int flags_) { + callback(uid, data, name.data(), name.size(), &value->value(), flags_); }, - immediate_notify != 0, - local_notify != 0); + flags); } void NT_RemoveEntryListener(unsigned int entry_listener_uid) { diff --git a/src/ntcore_cpp.cpp b/src/ntcore_cpp.cpp index 917b5aceb5..040452ed7e 100644 --- a/src/ntcore_cpp.cpp +++ b/src/ntcore_cpp.cpp @@ -66,11 +66,12 @@ void Flush() { */ unsigned int AddEntryListener(StringRef prefix, EntryListenerCallback callback, - bool immediate_notify, bool local_notify) { + unsigned int flags) { Notifier& notifier = Notifier::GetInstance(); - unsigned int uid = notifier.AddEntryListener(prefix, callback, local_notify); + unsigned int uid = notifier.AddEntryListener(prefix, callback, flags); notifier.Start(); - if (immediate_notify) Storage::GetInstance().NotifyEntries(prefix, callback); + if ((flags & NT_NOTIFY_IMMEDIATE) != 0) + Storage::GetInstance().NotifyEntries(prefix, callback); return uid; } diff --git a/src/tables/ITableListener.cpp b/src/tables/ITableListener.cpp new file mode 100644 index 0000000000..df1f273750 --- /dev/null +++ b/src/tables/ITableListener.cpp @@ -0,0 +1,9 @@ +#include "tables/ITableListener.h" + +#include "ntcore_c.h" + +void ITableListener::ValueChangedEx(ITable* source, llvm::StringRef key, + std::shared_ptr value, + unsigned int flags) { + ValueChanged(source, key, value, (flags & NT_NOTIFY_NEW) != 0); +}