[RSDK-9344] Use snappy for robot configuration compression #366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
acmorrow
force-pushed
the
RSDK-9344-compress-robot-config
branch
from
6f0bd53 to
d315660
Compare
acmorrow
commented
Dec 18, 2024
| encoded_cfg.len(), | ||
| compressed.len() | ||
| ); | ||
| self.set_blob(NVS_ROBOT_CONFIG_KEY, compressed.into())?; |
There was a problem hiding this comment.
One thing to contemplate here is what it might mean if we ever wanted to change this compression algorithm, or not do compression anymore. What would be the implications for upgrade? OTA?
acmorrow
commented
Dec 19, 2024
| Ok(()) | ||
| } | ||
|
|
||
| fn get_robot_configuration(&self) -> Result<RobotConfig, Self::Error> { | ||
| let robot_config = self.get_blob(NVS_ROBOT_CONFIG_KEY)?; | ||
| RobotConfig::decode(&robot_config[..]).map_err(NVSStorageError::NVSValueDecodeError) | ||
| let mut decompressed_robot_config = vec![]; |
There was a problem hiding this comment.
One thing we could do to harden this here would be that if we fail to decompress the buffer with snappy, that we then try to just feed the raw blob RobotConfig::decode. That'd protect against a situation where you had a non-compressed robot config in NVS, got OTA updated to a version that expected a compressed robot config, and then for some reason when you came back online were unable to reach app and the expectation would be to use the cached config.
acmorrow
force-pushed
the
RSDK-9344-compress-robot-config
branch
from
d315660 to
c937107
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
snap:App/part. size: 3,452,128/4,128,767 bytes, 83.61%snap:App/part. size: 3,487,808/4,128,767 bytes, 84.48%To test, I pasted a well-known large configuration (obtained by looking at the debug expansion of a fragment) into an attribute section of a sensor. Logs say:
So a 2,775 byte savings here, which isn't nothing given our NVS size bounds. The choice of snappy was somewhat arbitrary here, in that I tried it and it worked.