Skip to content

removed all instances of geneva - #3983

Open
noir-depresso wants to merge 10 commits into
UBC-Thunderbots:masterfrom
noir-depresso:remove-geneva
Open

noir-depresso wants to merge 10 commits into
UBC-Thunderbots:masterfrom
noir-depresso:remove-geneva

Conversation

@noir-depresso

Copy link
Copy Markdown
Contributor

Description

removed all instances of geneva and passes bazel build //...

Testing Done

Resolved Issues

resolves #3979

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@Andrewyx

Copy link
Copy Markdown
Contributor

Looks like this PR contains commits from another PR. Consider re-targeting or stacking PRs if blocking?

@Andrewyx Andrewyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

left some comments. Glad this cleanup is finally happening

Comment on lines +143 to +146
float current_draw = 3;

uint32 sequence_num = 5;
bool breakbeam_tripped = 6;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mark as reserved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need to as we will never be decoding old protos again

switch (power_control.geneva_slot())
{
case TbotsProto::Geneva::LEFT:
nanopb_control.geneva_slot = TbotsProto_Geneva_Slot_LEFT;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: extra NL

Comment thread src/local_clangd/BUILD.bazel Outdated
@@ -0,0 +1,6 @@
load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From this ticket or prev PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah it was from a test i did, i got rid of it


import "proto/robot_status_msg.proto";
import "proto/geneva_slot.proto";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: remove NL?


ChickerControl chicker = 1;
Geneva.Slot geneva_slot = 2;
ChickerControl chicker = 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mark as reserved?

Comment thread src/proto/primitive.proto

ChickerControl chicker = 1;
Geneva.Slot geneva_slot = 2;
ChickerControl chicker = 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mark as reserved

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.

Remove project geneva

3 participants