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

Deletes all existing building map entries if the id does not match the current served map, before saving #735

Merged
merged 2 commits into from
Aug 14, 2023

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Aug 11, 2023

What's new

Ensures that the map saved in db is always the one that is currently being served by the building map server.

This can be tested by starting an api-server instance, and changing the building map server,

# start api-server in a terminal
pnpm start

# serve office map, Ctrl+C, serve airport terminal map, repeat
ros2 run rmf_building_map_tools building_map_server WORKSPACE_DIR/install/rmf_demos_maps/share/rmf_demos_maps/office/office.building.yaml
ros2 run rmf_building_map_tools building_map_server WORKSPACE_DIR/install/rmf_demos_maps/share/rmf_demos_maps/airport_terminal/airport_terminal.building.yaml

Self-checks

  • I have prototyped this new feature (if necessary) on Figma
  • I'm familiar with and follow this Typescript guideline
  • I added unit-tests for new components
  • I tried testing edge cases
  • I tested the behavior of the components that interact with the backend, with an e2e test

…e current served map, before saving

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #735 (9066957) into main (5fcccd3) will increase coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #735      +/-   ##
==========================================
+ Coverage   54.64%   54.65%   +0.01%     
==========================================
  Files         263      263              
  Lines        6524     6528       +4     
  Branches      862      862              
==========================================
+ Hits         3565     3568       +3     
- Misses       2819     2820       +1     
  Partials      140      140              
Flag Coverage Δ
api-server 82.58% <75.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...kages/api-server/api_server/models/building_map.py 88.88% <75.00%> (-3.97%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@akash-roboticist akash-roboticist left a comment

Choose a reason for hiding this comment

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

Since other data is not affected in the db, I think this looks fine and will minimize issues due to map updates during commissioning phase.
Thanks Aaron!

@aaronchongth aaronchongth merged commit ac0ca6c into main Aug 14, 2023
3 checks passed
@aaronchongth aaronchongth deleted the fix/map-switching branch August 14, 2023 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants