Skip to content

Commit

Permalink
Uploaded .json settings bug fixups (#1082)
Browse files Browse the repository at this point in the history 
Resolved race condition between saveGlobal and saveOneFile modifying settings on shutdown. Previously, saveGlobal would overwrite the action of saveOneFile on a clean shutdown.
  • Loading branch information
@gerth2
gerth2 committed Dec 29, 2023
1 parent ece521c commit ef039da
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 31 deletions.
75 changes: 54 additions & 21 deletions photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,29 @@ private void addFile(PreparedStatement ps, String key, String value) throws SQLE
ps.setString(2, value);
}

// NOTE to Future Developers:
// These booleans form a mechanism to prevent saveGlobal() and
// saveOneFile() from stepping on each other's toes. Both write
// to the database on disk, and both write to the same keys, but
// they use different sources. Generally, if the user has done something
// to trigger saveOneFile() to get called, it implies they want that
// configuration, and not whatever is in RAM right now (which is what
// saveGlobal() uses to write). Therefor, once saveOneFile() is invoked,
// we record which entry was overwritten in the database and prevent
// overwriting it when saveGlobal() is invoked (likely by the shutdown
// that should almost almost almost happen right after saveOneFile() is
// invoked).
//
// In the future, this may not be needed. A better architecture would involve
// manipulating the RAM representation of configuration when new .json files
// are uploaded in the UI, and eliminate all other usages of saveOneFile().
// But, seeing as it's Dec 28 and kickoff is nigh, we put this here and moved on.
// Thank you for coming to my TED talk.
private boolean skipSavingHWCfg = false;
private boolean skipSavingHWSet = false;
private boolean skipSavingNWCfg = false;
private boolean skipSavingAPRTG = false;

private void saveGlobal(Connection conn) {
PreparedStatement statement1 = null;
PreparedStatement statement2 = null;
Expand All @@ -351,28 +374,34 @@ private void saveGlobal(Connection conn) {
// Replace this camera's row with the new settings
var sqlString = "REPLACE INTO global (filename, contents) VALUES " + "(?,?);";

statement1 = conn.prepareStatement(sqlString);
addFile(
statement1,
TableKeys.HARDWARE_SETTINGS,
JacksonUtils.serializeToString(config.getHardwareSettings()));
statement1.executeUpdate();
if (!skipSavingHWSet) {
statement1 = conn.prepareStatement(sqlString);
addFile(
statement1,
TableKeys.HARDWARE_SETTINGS,
JacksonUtils.serializeToString(config.getHardwareSettings()));
statement1.executeUpdate();
}

statement2 = conn.prepareStatement(sqlString);
addFile(
statement2,
TableKeys.NETWORK_CONFIG,
JacksonUtils.serializeToString(config.getNetworkConfig()));
statement2.executeUpdate();
statement2.close();

statement3 = conn.prepareStatement(sqlString);
addFile(
statement3,
TableKeys.HARDWARE_CONFIG,
JacksonUtils.serializeToString(config.getHardwareConfig()));
statement3.executeUpdate();
statement3.close();
if (!skipSavingNWCfg) {
statement2 = conn.prepareStatement(sqlString);
addFile(
statement2,
TableKeys.NETWORK_CONFIG,
JacksonUtils.serializeToString(config.getNetworkConfig()));
statement2.executeUpdate();
statement2.close();
}

if (!skipSavingHWCfg) {
statement3 = conn.prepareStatement(sqlString);
addFile(
statement3,
TableKeys.HARDWARE_CONFIG,
JacksonUtils.serializeToString(config.getHardwareConfig()));
statement3.executeUpdate();
statement3.close();
}

} catch (SQLException | IOException e) {
logger.error("Err saving global", e);
Expand Down Expand Up @@ -431,21 +460,25 @@ private boolean saveOneFile(String fname, Path path) {

@Override
public boolean saveUploadedHardwareConfig(Path uploadPath) {
skipSavingHWCfg = true;
return saveOneFile(TableKeys.HARDWARE_CONFIG, uploadPath);
}

@Override
public boolean saveUploadedHardwareSettings(Path uploadPath) {
skipSavingHWSet = true;
return saveOneFile(TableKeys.HARDWARE_SETTINGS, uploadPath);
}

@Override
public boolean saveUploadedNetworkConfig(Path uploadPath) {
skipSavingNWCfg = true;
return saveOneFile(TableKeys.NETWORK_CONFIG, uploadPath);
}

@Override
public boolean saveUploadedAprilTagFieldLayout(Path uploadPath) {
skipSavingAPRTG = true;
return saveOneFile(TableKeys.ATFL_CONFIG_FILE, uploadPath);
}

Expand Down
24 changes: 14 additions & 10 deletions photon-server/src/main/java/org/photonvision/server/RequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ public static void onSettingsImportRequest(Context ctx) {

if (ConfigManager.saveUploadedSettingsZip(tempFilePath.get())) {
ctx.status(200);
ctx.result("Successfully saved the uploaded settings zip, rebooting");
logger.info("Successfully saved the uploaded settings zip, rebooting");
ctx.result("Successfully saved the uploaded settings zip, rebooting...");
logger.info("Successfully saved the uploaded settings zip, rebooting...");
restartProgram();
} else {
ctx.status(500);
Expand Down Expand Up @@ -153,8 +153,9 @@ public static void onHardwareConfigRequest(Context ctx) {

if (ConfigManager.getInstance().saveUploadedHardwareConfig(tempFilePath.get().toPath())) {
ctx.status(200);
ctx.result("Successfully saved the uploaded hardware config");
logger.info("Successfully saved the uploaded hardware config");
ctx.result("Successfully saved the uploaded hardware config, rebooting...");
logger.info("Successfully saved the uploaded hardware config, rebooting...");
restartProgram();
} else {
ctx.status(500);
ctx.result("There was an error while saving the uploaded hardware config");
Expand Down Expand Up @@ -195,8 +196,9 @@ public static void onHardwareSettingsRequest(Context ctx) {

if (ConfigManager.getInstance().saveUploadedHardwareSettings(tempFilePath.get().toPath())) {
ctx.status(200);
ctx.result("Successfully saved the uploaded hardware settings");
logger.info("Successfully saved the uploaded hardware settings");
ctx.result("Successfully saved the uploaded hardware settings, rebooting...");
logger.info("Successfully saved the uploaded hardware settings, rebooting...");
restartProgram();
} else {
ctx.status(500);
ctx.result("There was an error while saving the uploaded hardware settings");
Expand Down Expand Up @@ -237,8 +239,9 @@ public static void onNetworkConfigRequest(Context ctx) {

if (ConfigManager.getInstance().saveUploadedNetworkConfig(tempFilePath.get().toPath())) {
ctx.status(200);
ctx.result("Successfully saved the uploaded network config");
logger.info("Successfully saved the uploaded network config");
ctx.result("Successfully saved the uploaded network config, rebooting...");
logger.info("Successfully saved the uploaded network config, rebooting...");
restartProgram();
} else {
ctx.status(500);
ctx.result("There was an error while saving the uploaded network config");
Expand Down Expand Up @@ -279,8 +282,9 @@ public static void onAprilTagFieldLayoutRequest(Context ctx) {

if (ConfigManager.getInstance().saveUploadedAprilTagFieldLayout(tempFilePath.get().toPath())) {
ctx.status(200);
ctx.result("Successfully saved the uploaded AprilTagFieldLayout");
logger.info("Successfully saved the uploaded AprilTagFieldLayout");
ctx.result("Successfully saved the uploaded AprilTagFieldLayout, rebooting...");
logger.info("Successfully saved the uploaded AprilTagFieldLayout, rebooting...");
restartProgram();
} else {
ctx.status(500);
ctx.result("There was an error while saving the uploaded AprilTagFieldLayout");
Expand Down

0 comments on commit ef039da

Please sign in to comment.