CameraServer: Remove NT-driven settings. (#467)

Unfortunately, due to the way NT synchronization is currently performed,
this has unexpected and undesirable behavior: when a dashboard (or any other
NT client) is left running between code restarts, when it reconnects, any
code settings will be overwritten by the NT synchronization process.  As
fixing this will require a fairly major NT change (and likely a user-visible
one), it's not desirable to do at this point in the year.

Instead, disable NT driven settings entirely (e.g. make the NT interface
publish only).  To emphasize the read-only nature of the NT values, attempts
to change the NT values will be immediately overridden by CameraServer.

To better inform users about the actual property names (e.g. for use in their
code), the "raw_" settings no longer have "raw_" removed from their names
(they are still placed in the "RawProperty" subtable).

This change also contains a couple of Java fixes:

* Make getInstance() thread-safe

* Properly synchronize access to m_tables between multiple threads.

* Use Hashtable instead of HashMap.
This commit is contained in:
Peter Johnson
2017-01-19 12:30:07 -07:00
committed by GitHub
parent 8f1b034b2f
commit b8537be219
2 changed files with 49 additions and 61 deletions

View File

@@ -171,7 +171,7 @@ static std::string PixelFormatToString(int pixelFormat) {
return "Unknown";
}
}
#if 0
static cs::VideoMode::PixelFormat PixelFormatFromString(llvm::StringRef str) {
if (str == "MJPEG" || str == "mjpeg" || str == "JPEG" || str == "jpeg")
return cs::VideoMode::PixelFormat::kMJPEG;
@@ -228,7 +228,7 @@ static cs::VideoMode VideoModeFromString(llvm::StringRef modeStr) {
return mode;
}
#endif
static std::string VideoModeToString(const cs::VideoMode& mode) {
std::string rv;
llvm::raw_string_ostream oss{rv};
@@ -261,9 +261,9 @@ static void PutSourcePropertyValue(ITable* table, const cs::VideoEvent& event,
llvm::SmallString<64> infoName;
if (llvm::StringRef{event.name}.startswith("raw_")) {
name = "RawProperty/";
name += llvm::StringRef{event.name}.substr(4);
name += event.name;
infoName = "RawPropertyInfo/";
infoName += llvm::StringRef{event.name}.substr(4);
infoName += event.name;
} else {
name = "Property/";
name += event.name;
@@ -427,6 +427,9 @@ CameraServer::CameraServer()
0x4fff, true};
// Listener for NetworkTable events
// We don't currently support changing settings via NT due to
// synchronization issues, so just update to current setting if someone
// else tries to change it.
llvm::SmallString<64> buf;
m_tableListener = nt::AddEntryListener(
Concatenate(kPublishName, "/", buf),
@@ -446,24 +449,16 @@ CameraServer::CameraServer()
relativeKey = relativeKey.substr(subKeyIndex + 1);
// handle standard names
llvm::SmallString<64> propNameBuf;
llvm::StringRef propName;
if (relativeKey == "mode") {
if (!value->IsString()) return;
auto mode = VideoModeFromString(value->GetString());
if (mode.pixelFormat == cs::VideoMode::PixelFormat::kUnknown ||
!sourceIt->second.SetVideoMode(mode)) {
// reset to current mode
nt::SetEntryValue(key, nt::Value::MakeString(VideoModeToString(
sourceIt->second.GetVideoMode())));
}
// reset to current mode
nt::SetEntryValue(key, nt::Value::MakeString(VideoModeToString(
sourceIt->second.GetVideoMode())));
return;
} else if (relativeKey.startswith("Property/")) {
propName = relativeKey.substr(9);
} else if (relativeKey.startswith("RawProperty/")) {
propNameBuf = "raw_";
propNameBuf += relativeKey.substr(12);
propName = propNameBuf.str();
propName = relativeKey.substr(12);
} else {
return; // ignore
}
@@ -474,17 +469,14 @@ CameraServer::CameraServer()
case cs::VideoProperty::kNone:
return;
case cs::VideoProperty::kBoolean:
if (!value->IsBoolean()) return;
property.Set(value->GetBoolean() ? 1 : 0);
nt::SetEntryValue(key, nt::Value::MakeBoolean(property.Get() != 0));
return;
case cs::VideoProperty::kInteger:
case cs::VideoProperty::kEnum:
if (!value->IsDouble()) return;
property.Set(static_cast<int>(value->GetDouble()));
nt::SetEntryValue(key, nt::Value::MakeDouble(property.Get()));
return;
case cs::VideoProperty::kString:
if (!value->IsString()) return;
property.SetString(value->GetString());
nt::SetEntryValue(key, nt::Value::MakeString(property.GetString()));
return;
default:
return;

View File

@@ -25,7 +25,7 @@ import edu.wpi.first.wpilibj.networktables.NetworkTable;
import edu.wpi.first.wpilibj.networktables.NetworkTablesJNI;
import edu.wpi.first.wpilibj.tables.ITable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Hashtable;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -49,7 +49,7 @@ public class CameraServer {
/**
* Get the CameraServer instance.
*/
public static CameraServer getInstance() {
public static synchronized CameraServer getInstance() {
if (server == null) {
server = new CameraServer();
}
@@ -57,12 +57,12 @@ public class CameraServer {
}
private String m_primarySourceName;
private HashMap<String, VideoSource> m_sources;
private HashMap<String, VideoSink> m_sinks;
private HashMap<Integer, ITable> m_tables; // indexed by source handle
private ITable m_publishTable;
private VideoListener m_videoListener; //NOPMD
private int m_tableListener; //NOPMD
private final Hashtable<String, VideoSource> m_sources;
private final Hashtable<String, VideoSink> m_sinks;
private final Hashtable<Integer, ITable> m_tables; // indexed by source handle
private final ITable m_publishTable;
private final VideoListener m_videoListener; //NOPMD
private final int m_tableListener; //NOPMD
private int m_nextPort;
private String[] m_addresses;
@@ -93,11 +93,6 @@ public class CameraServer {
return "mjpg:http://" + address + ":" + port + "/?action=stream";
}
@SuppressWarnings("JavadocMethod")
private synchronized ITable getSourceTable(int source) {
return m_tables.get(source);
}
@SuppressWarnings({"JavadocMethod", "PMD.AvoidUsingHardCodedIP"})
private synchronized String[] getSinkStreamValues(int sink) {
// Ignore all but MjpegServer
@@ -290,8 +285,8 @@ public class CameraServer {
String name;
String infoName;
if (event.name.startsWith("raw_")) {
name = "RawProperty/" + event.name.substring(4);
infoName = "RawPropertyInfo/" + event.name.substring(4);
name = "RawProperty/" + event.name;
infoName = "RawPropertyInfo/" + event.name;
} else {
name = "Property/" + event.name;
infoName = "PropertyInfo/" + event.name;
@@ -335,9 +330,9 @@ public class CameraServer {
@SuppressWarnings({"JavadocMethod", "PMD.UnusedLocalVariable"})
private CameraServer() {
m_sources = new HashMap<String, VideoSource>();
m_sinks = new HashMap<String, VideoSink>();
m_tables = new HashMap<Integer, ITable>();
m_sources = new Hashtable<String, VideoSource>();
m_sinks = new Hashtable<String, VideoSink>();
m_tables = new Hashtable<Integer, ITable>();
m_publishTable = NetworkTable.getTable(kPublishName);
m_nextPort = kBasePort;
m_addresses = new String[0];
@@ -359,9 +354,7 @@ public class CameraServer {
case kSourceCreated: {
// Create subtable for the camera
ITable table = m_publishTable.getSubTable(event.name);
synchronized (this) {
m_tables.put(event.sourceHandle, table);
}
m_tables.put(event.sourceHandle, table);
table.putString("source", makeSourceValue(event.sourceHandle));
table.putString("description",
CameraServerJNI.getSourceDescription(event.sourceHandle));
@@ -373,7 +366,7 @@ public class CameraServer {
break;
}
case kSourceDestroyed: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
table.putString("source", "");
table.putStringArray("streams", new String[0]);
@@ -382,7 +375,7 @@ public class CameraServer {
break;
}
case kSourceConnected: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
// update the description too (as it may have changed)
table.putString("description",
@@ -392,42 +385,42 @@ public class CameraServer {
break;
}
case kSourceDisconnected: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
table.putBoolean("connected", false);
}
break;
}
case kSourceVideoModesUpdated: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
table.putStringArray("modes", getSourceModeValues(event.sourceHandle));
}
break;
}
case kSourceVideoModeChanged: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
table.putString("mode", videoModeToString(event.mode));
}
break;
}
case kSourcePropertyCreated: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
putSourcePropertyValue(table, event, true);
}
break;
}
case kSourcePropertyValueUpdated: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
putSourcePropertyValue(table, event, false);
}
break;
}
case kSourcePropertyChoicesUpdated: {
ITable table = getSourceTable(event.sourceHandle);
ITable table = m_tables.get(event.sourceHandle);
if (table != null) {
String[] choices = CameraServerJNI.getEnumPropertyChoices(event.propertyHandle);
table.putStringArray("PropertyInfo/" + event.name + "/choices", choices);
@@ -450,8 +443,11 @@ public class CameraServer {
}, 0x4fff, true);
// Listener for NetworkTable events
// We don't currently support changing settings via NT due to
// synchronization issues, so just update to current setting if someone
// else tries to change it.
m_tableListener = NetworkTablesJNI.addEntryListener(kPublishName + "/",
(uid, key, value, flags) -> {
(uid, key, eventValue, flags) -> {
String relativeKey = key.substring(kPublishName.length() + 1);
// get source (sourceName/...)
@@ -471,16 +467,13 @@ public class CameraServer {
// handle standard names
String propName;
if (relativeKey.equals("mode")) {
VideoMode mode = videoModeFromString((String) value);
if (mode.pixelFormat == PixelFormat.kUnknown || !source.setVideoMode(mode)) {
// reset to current mode
NetworkTablesJNI.putString(key, videoModeToString(source.getVideoMode()));
}
// reset to current mode
NetworkTablesJNI.putString(key, videoModeToString(source.getVideoMode()));
return;
} else if (relativeKey.startsWith("Property/")) {
propName = relativeKey.substring(9);
} else if (relativeKey.startsWith("RawProperty/")) {
propName = "raw_" + relativeKey.substring(12);
propName = relativeKey.substring(12);
} else {
return; // ignore
}
@@ -491,14 +484,17 @@ public class CameraServer {
case kNone:
return;
case kBoolean:
property.set(((Boolean) value).booleanValue() ? 1 : 0);
// reset to current setting
NetworkTablesJNI.putBoolean(key, property.get() != 0);
return;
case kInteger:
case kEnum:
property.set(((Double) value).intValue());
// reset to current setting
NetworkTablesJNI.putDouble(key, property.get());
return;
case kString:
property.setString((String) value);
// reset to current setting
NetworkTablesJNI.putString(key, property.getString());
return;
default:
return;