Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Uploaded .json settings bug fixups #1082

Merged
merged 2 commits into from
Dec 29, 2023
Merged

Uploaded .json settings bug fixups #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
589d7a7
Resolved race condition between saveGlobal and saveOneFile modifying …
gerth2 Dec 29, 2023
8116cc6
phat comments
gerth2 Dec 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 54 additions & 21 deletions photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java
View file
Open in desktop
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
Open in desktop
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