Skip to content

Conversation

@arjo129
Copy link
Member

@arjo129 arjo129 commented Sep 22, 2025

Implemented feature

Adds support to export an occupancy grid to a PNG so other external tools may consume the exported occupancy grid.

Implementation description

GenAI Use

We follow OSRA's policy on GenAI tools

  • I used a GenAI tool in this PR.
  • I did not use GenAI

Generated-by:

We seem to have removed any way of exporting occupancy grids from the
site editor. This commit adds a small UI to do said task.

Signed-off-by: Arjo Chakravarty <[email protected]>
This commit exports the occupancy grid to a PNG in a user selected target directory.

Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
@mxgrey mxgrey moved this from Inbox to In Progress in PMC Board Sep 23, 2025
@arjo129 arjo129 marked this pull request as ready for review September 23, 2025 06:52
@mxgrey
Copy link
Collaborator

mxgrey commented Sep 23, 2025

Just a heads up, this is going to run into some merge conflicts with #373 which does a refactor of calculate_grid. I think overall it will be beneficial for this PR since you can introduce an ExportGridRequest struct and a handle_export_grid_request system instead of adding the trigger_save field to CalculateGrid. That will provide a better separation of concerns.

@arjo129
Copy link
Member Author

arjo129 commented Sep 23, 2025

Should I retarget #373 or wait for that to be merged?

@mxgrey
Copy link
Collaborator

mxgrey commented Sep 23, 2025

Definitely wait for the merge since we'll be doing a squash merge which will bork the git history if you merge in #373 as-is into this branch.

@mxgrey
Copy link
Collaborator

mxgrey commented Sep 24, 2025

The other PR has been merged. The conflicts for this PR are now visible.

@mxgrey mxgrey moved this from In Progress to In Review in PMC Board Oct 7, 2025
@mxgrey mxgrey moved this from In Review to In Progress in PMC Board Oct 7, 2025
@mxgrey mxgrey removed the request for review from luca-della-vedova October 7, 2025 01:29
@mxgrey mxgrey marked this pull request as draft October 7, 2025 01:29
@arjo129 arjo129 self-assigned this Nov 4, 2025
You should be able to simply go `File` > `Export Occupancy Grid` and the occupancy grids will be generated directly.

Signed-off-by: Arjo <[email protected]>
@arjo129
Copy link
Member Author

arjo129 commented Nov 17, 2025

Works on desktops. You can simply click File>Export Occupancy as shown in the video below. It is currently disabled on the web as we don't support IO there.

output2.mp4

@arjo129 arjo129 marked this pull request as ready for review November 17, 2025 05:51
@aaronchongth aaronchongth moved this from In Progress to In Review in PMC Board Nov 18, 2025
}

fn handle_calculate_grid_request(
fn handle_export_request(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally not too excited about this function since it's pretty much the exact same as the one below, with one line of difference in "what we do after calculate_grid is over".

My preference would be to refactor them into a single function, on top of my head I can think of a few ways:

  • Refactor the two input events ExportGridRequest and CalculateGridRequest into a single event that has an additional field that says "what do we do with the output", we could match it and either send the saving event (your function) or send the replan event (the original function).
  • Make the function generic, perhaps as a generic parameter a closure that takes World and decides what to do after the grid is exported?
  • Bite the bullet and make this a workflow, which is quite a good fit for "sequence of operations that we need to run one after the other", so we could have the first node of the workflow that handles the calculate_grid request in a common service, then two workflows, one to calculate the occupancy grid and replan and the other one to calculate the occupancy grid and send a file dialog request.

First is quick and easy, second is possibly not a great idea, third would probably be the best but a bit more work to implement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had done 1 earlier (before having to rewrite the entire PR): #389 (comment)

2 will make the code unnecessarily complex.

3 is a fair deal more of work but probably the correct way to handle it.

@mxgrey mxgrey moved this from In Review to In Progress in PMC Board Dec 30, 2025
@arjo129 arjo129 moved this from In Progress to Blocked in PMC Board Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Blocked

Development

Successfully merging this pull request may close these issues.

4 participants