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.

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

[Feature] Reserve memory region for persistent system settings. #1622

Closed
laurensvalk opened this issue Apr 29, 2024 · 10 comments
Closed

[Feature] Reserve memory region for persistent system settings. #1622

laurensvalk opened this issue Apr 29, 2024 · 10 comments
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: bluetooth Issues involving bluetooth topic: imu Issues related to IMU/gyro/accelerometer

Comments

@laurensvalk
Copy link
Member

laurensvalk commented Apr 29, 2024

Is your feature request related to a problem? Please describe.
We have a persistent memory section for user data. We might need one for system settings as well.

We can either reduce the existing user data size, or reduce program size instead.

Taking it out of the user data has the benefit that there is no breaking change for user programs already on SPIKE hubs when they upgrade the firmware. I've also not seen many projects that use large amounts of persistent data anyway.

Persistent data we could consider reserving space for:

@laurensvalk laurensvalk added enhancement New feature or request triage Issues that have not been triaged yet topic: bluetooth Issues involving bluetooth software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: imu Issues related to IMU/gyro/accelerometer and removed triage Issues that have not been triaged yet labels Apr 29, 2024
@laurensvalk
Copy link
Member Author

laurensvalk commented May 3, 2024

One issue when starting this is that the region could contain any random data to begin with on Spike Hubs. On Technic/Boost/City it is erased on update.

Do we want to clear it on first boot on SPIKE hubs? Should we erase user programs too?

@dlech
Copy link
Member

dlech commented May 3, 2024

Maybe on those hubs, we could use a magic number or checksum to tell if the persistent data is valid or not.

@afarago
Copy link

afarago commented May 4, 2024

Could we consider having 3 BT modes?

  • BT on open (current)
  • BT off
  • BT on secure - only connect to last connected MAC (after mode 1)

Though kids are nice it is a disaster if during the competition anyone can connect to your hub when doing the programming / testing part.

(You know the analogy - in the IoT abbreviation the letter "s" stands for security)

@laurensvalk
Copy link
Member Author

Thanks @afarago --- it might be best to add that to #1615

This issue is more about the generics of storing persistent data 🙂

@afarago
Copy link

afarago commented May 4, 2024

Apologies, I was going to, but accidentally left the wrong ticket open on my phone.
note to myself: never comment on github before the morning coffee.

I will add it there, please feel free to remove my comment(s) from here.

@laurensvalk
Copy link
Member Author

Oh, don't worry! Please keep the comments coming before your morning coffee, those are often the best 😄

@laurensvalk
Copy link
Member Author

I would like to avoid a repeat of #744, so maybe we should just take the space needed for persistent storage out of the user data area.

On the other platforms this shouldn't matter, since all user data gets erased during a firmware update anyway.

@dlech
Copy link
Member

dlech commented Jun 5, 2024

On the other platforms this shouldn't matter, since all user data gets erased during a firmware update anyway.

Perhaps we could emulate this on the SPIKE hubs by including a hash of the data plus the firmware version (or a data layout version that only changes when the layout changes, like this proposed change here - this would need to include PBSYS_APP_HUB_FEATURE_FLAGS in the hash too). If the hash is bad, the write over the memory with 0s before reading the first time.

@laurensvalk
Copy link
Member Author

Related / maybe addressable at the same time #1496:

(...) a program is not really able to tell if the contents of the persistent storage are valid for the given program or not, because new programs are possible to install without clearing the storage, which makes it hard to use it safely (...)

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Jun 6, 2024
This ensures we don't run broken programs when the memory layout changes. See pybricks/support#1622
@laurensvalk
Copy link
Member Author

The mechanics for this have been implemented. There is just one setting for now, but we can add more with every release since the settings are erased after an update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: bluetooth Issues involving bluetooth topic: imu Issues related to IMU/gyro/accelerometer
Projects
None yet
Development

No branches or pull requests

3 participants