Auto-sync Cockpit settings with BlueOS #701

rafaellehmkuhl · 2024-01-24T21:18:09Z

This is a major patch, that changes how Cockpit works with its settings fundamentally.

With it, instead of having the browser localStorage as the source of truth for its settings, Cockpit now uses the backend (provided here by the bag-of-holdings service on BlueOS) to sync Cockpit's storage with BlueOS.

There's a pipeline, where Cockpit first tries to fetch its settings from BlueOS. We call it the initial sync. If the values differ, the user will be asked which one they want to keep.

After a successful sync, it then starts to use its own variables, now synced, as the new source of truth. We do that so the settings are always stored in the vehicle, but in the case the user changes something in Cockpit, this change is pushed to the BlueOS storage as the new (updated) value.

In case something goes wrong during the initial fetch (e.g.: the vehicle was not connected yet), Cockpit will keep trying to sync. When the vehicle connects, if the user has made no change to the settings, nothing will happen, but if the user has made some change during this interval, a popup will open asking if the user prefers to keep the changes made in Cockpit or prefers to use the value stored on BlueOS. We do that so we don't overwrite changes explicitly made by the user.

~~I'm opening this as a draft because I'm still testing to see if everything works correctly, but the code structure is ready and there won't be major changes.~~

Fix #666.
~~To be merged after #559.~~

goasChris · 2024-04-30T11:28:41Z

Hello Rafael :) I was hoping to include this PR in my testing branch, to give it some time in the water, but there are a few merge conflicts against current master, for example: #857

I'm a little bit torn, as I tried to fix it up, but then I end up with some parts of the code that is now using the old useStorage() method, and I am not familiar enough with the codebase to tell if this is going to be okay'ish or if it could cause trouble.

Any chance you could rebase this on current master when you have some spare time? I know it is not a small ask, so no rush, just wanted to let you know that this currently making it a bit difficult for me to test :) Much love, thank you for all your hard work <3

rafaellehmkuhl · 2024-04-30T12:28:42Z

Hello Rafael :) I was hoping to include this PR in my testing branch, to give it some time in the water, but there are a few merge conflicts against current master, for example: #857

I'm a little bit torn, as I tried to fix it up, but then I end up with some parts of the code that is now using the old useStorage() method, and I am not familiar enough with the codebase to tell if this is going to be okay'ish or if it could cause trouble.

Any chance you could rebase this on current master when you have some spare time? I know it is not a small ask, so no rush, just wanted to let you know that this currently making it a bit difficult for me to test :) Much love, thank you for all your hard work <3

Hey Chris! Everything cool?

Sure! Let me take a look at that.
This PR is actually almost ready, I just need to fix some edge-cases.
Will rebase this now.

rafaellehmkuhl · 2024-04-30T12:56:22Z

@goasChris just rebased it. Can you take a look?

goasChris · 2024-04-30T13:01:39Z

Yep, no problems pulling in the PR now :) Well, besides failing CI/autotests, but I'm not too concerned about that, hehe. Thank you very much!

rafaellehmkuhl · 2024-04-30T14:52:18Z

Yep, no problems pulling in the PR now :) Well, besides failing CI/autotests, but I'm not too concerned about that, hehe. Thank you very much!

Nice! No problem! Also, just fixed the CI, it was an unused import left in the code.

Let us know if you have anything more, and feel free to open issues around problems and/or feature requests, so we can track and discuss them!

Edit: strangely, there's a test for widget-loading failing, even if this part of the code wasn't touched here. Will investigate more.

goasChris · 2024-05-16T08:02:05Z

I've tried to debug why the tests are failing, but not being very familiar with ts, bun or vitest for that matter, I essentially just ended up setting a higher timeout, and printing a bunch of stuff. Unsure if its useful, or if its just noise.

stdout | src/tests/libs/widgets-loader.test.ts > Test widgets exist
Starting enum_to_files_checker...
Importing: @/components/widgets/Attitude.vue at 0 ms
Importing: @/components/widgets/Compass.vue at 0 ms
Importing: @/components/widgets/DepthHUD.vue at 0 ms
Importing: @/components/widgets/CompassHUD.vue at 1 ms
Importing: @/components/widgets/IFrame.vue at 1 ms
Importing: @/components/widgets/ImageView.vue at 1 ms
Importing: @/components/widgets/Map.vue at 1 ms
Importing: @/components/widgets/MiniWidgetsBar.vue at 1 ms
Importing: @/components/widgets/URLVideoPlayer.vue at 1 ms
Importing: @/components/widgets/VideoPlayer.vue at 1 ms
Importing: @/components/widgets/VirtualHorizon.vue at 1 ms
...
stderr | src/tests/libs/widgets-loader.test.ts > Test widgets exist
Failed to import: @/components/widgets/ImageView.vue at 1292 ms TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".css" for /home/chris/repos/cockpit/node_modules/vuetify/lib/components/VBtn/VBtn.css
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
    at defaultLoad (node:internal/modules/esm/load:143:22)
    at async nextLoad (node:internal/modules/esm/hooks:865:22)
    at async Hooks.load (node:internal/modules/esm/hooks:448:20)
    at async handleMessage (node:internal/modules/esm/worker:196:18) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}
Failed to import: @/components/widgets/URLVideoPlayer.vue at 1294 ms TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".css" for /home/chris/repos/cockpit/node_modules/vuetify/lib/components/VBtn/VBtn.css
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
    at defaultLoad (node:internal/modules/esm/load:143:22)
    at async nextLoad (node:internal/modules/esm/hooks:865:22)
    at async Hooks.load (node:internal/modules/esm/hooks:448:20)
    at async handleMessage (node:internal/modules/esm/worker:196:18) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

stdout | src/tests/libs/widgets-loader.test.ts > Test mini-widgets exist
Starting enum_to_files_checker...
Importing: @/components/mini-widgets/ArmerButton.vue at 0 ms
Importing: @/components/mini-widgets/BaseCommIndicator.vue at 0 ms
Importing: @/components/mini-widgets/BatteryIndicator.vue at 0 ms
Importing: @/components/mini-widgets/ChangeAltitudeCommander.vue at 0 ms
Importing: @/components/mini-widgets/DepthIndicator.vue at 0 ms
Importing: @/components/mini-widgets/RelativeAltitudeIndicator.vue at 0 ms
Importing: @/components/mini-widgets/TakeoffLandCommander.vue at 0 ms
Importing: @/components/mini-widgets/VeryGenericIndicator.vue at 0 ms
Importing: @/components/mini-widgets/JoystickCommIndicator.vue at 0 ms
Importing: @/components/mini-widgets/MiniVideoRecorder.vue at 0 ms
Importing: @/components/mini-widgets/ModeSelector.vue at 0 ms
Importing: @/components/mini-widgets/SatelliteIndicator.vue at 0 ms
Importing: @/components/mini-widgets/ViewSelector.vue at 0 ms

stderr | src/tests/libs/widgets-loader.test.ts > Test mini-widgets exist
Failed to import: @/components/mini-widgets/JoystickCommIndicator.vue at 127 ms Error: This browser does not support gamepad API.
    at new a (/home/chris/repos/cockpit/node_modules/gamepad.js/gamepad.js:1:13751)
    at <instance_members_initializer> (/home/chris/repos/cockpit/src/libs/joystick/manager.ts:75:21)
    at new JoystickManager (/home/chris/repos/cockpit/src/libs/joystick/manager.ts:78:14)
    at <static_initializer> (/home/chris/repos/cockpit/src/libs/joystick/manager.ts:72:21)
    at /home/chris/repos/cockpit/src/libs/joystick/manager.ts:70:3
    at async VitestRunner.directRequest (file:///home/chris/repos/cockpit/node_modules/vitest/dist/chunk-vite-node-client.115caed2.mjs:188:5)
    at async VitestRunner.cachedRequest (file:///home/chris/repos/cockpit/node_modules/vitest/dist/chunk-vite-node-client.115caed2.mjs:88:12)
    at async _VitestMocker.request (file:///home/chris/repos/cockpit/node_modules/vitest/dist/chunk-vite-node-client.115caed2.mjs:110:21)
    at async /home/chris/repos/cockpit/src/components/mini-widgets/JoystickCommIndicator.vue:5:31
    at async VitestRunner.directRequest (file:///home/chris/repos/cockpit/node_modules/vitest/dist/chunk-vite-node-client.115caed2.mjs:188:5)

rafaellehmkuhl · 2024-05-16T11:42:27Z

I've tried to debug why the tests are failing, but not being very familiar with ts, bun or vitest for that matter, I essentially just ended up setting a higher timeout, and printing a bunch of stuff. Unsure if its useful, or if its just noise.

The strange part is that none of those things were modified...

BTW have you been able to use the PR?

goasChris · 2024-05-16T13:18:15Z

I unfortunately have not been able to test it, cus the only way I know how to fairly easily get the extension input one of our bots is to rely on CI (which won't build heh)

rafaellehmkuhl · 2024-06-26T18:59:14Z

@ArturoManzoli @patrickelectric ~~there's some strange CI problem happening that I'm investigating, but the PR is already done and ready for review.~~

Fixed.

goasChris · 2024-06-27T06:56:45Z

@rafaellehmkuhl You are the best! Thank you for all the good work. I have to admit I am extremely curious how you fixed CI, since I failed to do so myself.

rafaellehmkuhl · 2024-06-27T11:29:21Z

@rafaellehmkuhl You are the best! Thank you for all the good work. I have to admit I am extremely curious how you fixed CI, since I failed to do so myself.

I actually did nothing 😂

I wrote the comment before the CI finished running and afterwards realized it was fixed by itself.

Probably a bug on some dynamically versioned package.

patrickelectric · 2024-06-27T19:48:50Z

src/composables/settingsSyncer.ts

+        { text: 'Use the value from BlueOS', action: () => preferBlueOs() },
+        { text: "Keep Cockpit's value", action: () => preferCockpit() },


Suggested change

{ text: 'Use the value from BlueOS', action: () => preferBlueOs() },

{ text: "Keep Cockpit's value", action: () => preferCockpit() },

{ text: 'Use the value from BlueOS', action: preferBlueOs },

{ text: "Keep Cockpit's value", action: preferCockpit },

This should work, right ?

…evels

The `useBlueOsStorage` composable will keep a setting in sync between local storage and BlueOS. The initial value will be the one stored on BlueOS. If we fail to get the value from BlueOS on the first seconds after boot, we will ask the user if they prefer to use the value stored locally or the value stored on BlueOS. Once everything is in sync, if the local value changes, it will update the value on BlueOS. In resume, the initial source of truth is BlueOS, and once everything is in sync, the source of truth is the local value.

…browser storage)

This can be used to check if the path was not set, and should then be set for the first time.

This is now done automatically by the settings syncer.

This prevents bloat of the user's storage as well as problematic behaviors when the user changes devices.

Prevents updates on one part of the code that brakes the other that is not connected to it.

We were using it to check if the widget was ever mounted, so it already satisfies the requirements. Updating it on every mount was causing the profile to change all the time, which makes syncing dificult.

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 94745d2 to 79a095c Compare February 1, 2024 14:04

This was referenced Feb 2, 2024

Improve video download pipeline to guarantee (as much as possible) that recorded videos are not lost #715

Closed

Store video recording on the vehicle instead of the top-side computer #719

Open

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 79a095c to b742866 Compare February 7, 2024 19:02

rafaellehmkuhl added the docs-needed Change needs to be documented label Feb 21, 2024

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from b742866 to 886e307 Compare February 21, 2024 17:29

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 886e307 to 9d76664 Compare April 30, 2024 12:56

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 9d76664 to 76493d9 Compare April 30, 2024 14:50

rafaellehmkuhl marked this pull request as ready for review May 16, 2024 11:32

rafaellehmkuhl marked this pull request as draft May 16, 2024 11:34

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch 4 times, most recently from 2bd32a0 to 77ae20a Compare June 10, 2024 16:36

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch 3 times, most recently from a309deb to 374ba77 Compare June 20, 2024 17:09

patrickelectric self-requested a review June 21, 2024 12:17

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch 4 times, most recently from 351e730 to 781cc84 Compare June 26, 2024 18:48

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 781cc84 to b9c64ce Compare June 26, 2024 18:56

rafaellehmkuhl requested a review from ArturoManzoli June 26, 2024 18:56

rafaellehmkuhl marked this pull request as ready for review June 26, 2024 18:56

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from b9c64ce to 7c613da Compare June 26, 2024 18:58

patrickelectric reviewed Jun 27, 2024

View reviewed changes

patrickelectric approved these changes Jun 27, 2024

View reviewed changes

rafaellehmkuhl added 10 commits June 27, 2024 16:58

alert-store: Create abstracted functions for pushing specific alert l…

0a77a4b

…evels

vehicle-settings: Store vehicle settings inside it (instead of local …

e9826a3

…browser storage)

libs: blueos: Use path access on bag-of-holding to simplify queries

fe1db9b

libs: blueos: Add specific error for when the path is not available

4425688

This can be used to check if the path was not set, and should then be set for the first time.

joystick: Remove options to import/export mappings from/to vehicle

1a73167

This is now done automatically by the settings syncer.

configuration-logs: Remove unnecessary usage of browser storage

dd64061

This prevents bloat of the user's storage as well as problematic behaviors when the user changes devices.

configuration-logs: Remove leftover console logs

cc812a5

configuration-logs: Remove redundant code

ae75f54

Prevents updates on one part of the code that brakes the other that is not connected to it.

widgets: Replace timesMounted with everMounted on the manager vars

1aedae6

We were using it to check if the widget was ever mounted, so it already satisfies the requirements. Updating it on every mount was causing the profile to change all the time, which makes syncing dificult.

rafaellehmkuhl force-pushed the auto-sync-cockpit-settings branch from 7c613da to 1aedae6 Compare June 27, 2024 19:59

patrickelectric merged commit 78365a5 into bluerobotics:master Jun 27, 2024
8 checks passed

rafaellehmkuhl deleted the auto-sync-cockpit-settings branch June 27, 2024 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-sync Cockpit settings with BlueOS #701

Auto-sync Cockpit settings with BlueOS #701

rafaellehmkuhl commented Jan 24, 2024 •

edited

Loading

goasChris commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024

goasChris commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024 •

edited

Loading

goasChris commented May 16, 2024

rafaellehmkuhl commented May 16, 2024

goasChris commented May 16, 2024 •

edited

Loading

rafaellehmkuhl commented Jun 26, 2024 •

edited

Loading

goasChris commented Jun 27, 2024

rafaellehmkuhl commented Jun 27, 2024

patrickelectric Jun 27, 2024

		{ text: 'Use the value from BlueOS', action: () => preferBlueOs() },
		{ text: "Keep Cockpit's value", action: () => preferCockpit() },

Auto-sync Cockpit settings with BlueOS #701

Auto-sync Cockpit settings with BlueOS #701

Conversation

rafaellehmkuhl commented Jan 24, 2024 • edited Loading

goasChris commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024

goasChris commented Apr 30, 2024

rafaellehmkuhl commented Apr 30, 2024 • edited Loading

goasChris commented May 16, 2024

rafaellehmkuhl commented May 16, 2024

goasChris commented May 16, 2024 • edited Loading

rafaellehmkuhl commented Jun 26, 2024 • edited Loading

goasChris commented Jun 27, 2024

rafaellehmkuhl commented Jun 27, 2024

patrickelectric Jun 27, 2024

Choose a reason for hiding this comment

rafaellehmkuhl commented Jan 24, 2024 •

edited

Loading

rafaellehmkuhl commented Apr 30, 2024 •

edited

Loading

goasChris commented May 16, 2024 •

edited

Loading

rafaellehmkuhl commented Jun 26, 2024 •

edited

Loading