Issue #85: Set initial storage level #87
Conversation
LorenzzoQM
force-pushed
the
feature/85_set_initial_data_storage
branch
2 times, most recently
from
5bca45f to
418cc1a
Compare
| """ | ||
| self.storageUnit = simpleStorageUnit.SimpleStorageUnit() | ||
| self.storageUnit.ModelTag = "storageUnit" + self.satellite.id | ||
| self.storageUnit.storageCapacity = dataStorageCapacity # bits | ||
| self.storageUnit.addDataNodeToModel(self.instrument.nodeDataOutMsg) | ||
| self.storageUnit.addDataNodeToModel(self.transmitter.nodeDataOutMsg) | ||
| self.storageUnitValidCheck = storageUnitValidCheck | ||
| self.storageUnit.setDataBuffer(setStorageInit) |
There was a problem hiding this comment.
Does BSK properly handle range checking (negative storage? setStorageInit>dataStorageCapacity?)? Is there any sort of warning if that's violated? Or do you not think that's necessary
There was a problem hiding this comment.
The storage module doesn't allow the storage to go lower than zero or higher than the limit using the setDataBuffer. So, in case the data amount passed to setDataBuffer violates one of those cases, the module won't perform the addition/subtraction. Since these cases won't happen, I don't think warnings are really necessary.
| @default_args(dataStorageCapacity=20 * 8e6, storageUnitValidCheck=True) | ||
| @default_args( | ||
| dataStorageCapacity=20 * 8e6, storageUnitValidCheck=True, setStorageInit=0 | ||
| ) | ||
| def _set_storage_unit( | ||
| self, | ||
| dataStorageCapacity: int, | ||
| priority: int = 699, | ||
| storageUnitValidCheck: bool = True, |
There was a problem hiding this comment.
Does this still work how we wanted with the change to integers? I think so, but not sure if the floating point error was part of that
There was a problem hiding this comment.
Good point. The storage is not supposed to go over its limit anymore due to changes in the baseStorageUnit module (using integers and how it checks for free space). But storageValidCheck will still check if the storage unit is at its maximum capacity. So, I believe it still has the expected behavior.
| def _set_storage_unit( | ||
| self, | ||
| dataStorageCapacity: int, | ||
| priority: int = 699, | ||
| storageUnitValidCheck: bool = True, | ||
| setStorageInit=0, |
There was a problem hiding this comment.
Let's add a type hint (i.e. storageInit: int=0) and change the name to follow the convention of not including set in the variable names.
LorenzzoQM
force-pushed
the
feature/85_set_initial_data_storage
branch
from
418cc1a to
ff2f976
Compare
|
lgtm! |
Description
Closes #85
An extra argument was added to the _set_storage_unit method in the ContinuousImagingDynModel class to allowing the user to specify the initial storage level.
How should this pull request be reviewed?
Type of change
How Has This Been Tested?
pytest --cov bsk_rl/envs/general_satellite_tasking --cov-report term-missing tests/unittestpytest --cov bsk_rl/envs/general_satellite_tasking --cov-report term-missing tests/integrationTest Configuration
Checklist:
Issue #XXX: Messageand have a useful message