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

[core] Added ComponentType::RemoteID #2413

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

dakejahl
Copy link
Contributor

@dakejahl dakejahl commented Oct 4, 2024

  • Removed unused duplicate enum class in system.h Can we consolidate System::ComponentType with Mavsdk::ComponentType?
  • Added ComponentType::RemoteID -- I'm transitioning RemoteIDTransmitter to MAVSDK
  • Removed ComponentType::Custom to reduce confusion since it was defaulting to MAV_TYPE_GENERIC
  • Added Mavsdk::Configuration constructor for ComponentType::RemoteId

@dakejahl
Copy link
Contributor Author

dakejahl commented Oct 4, 2024

Nevermind it looks like it didn't do a clean build and ComponentType in System is indeed used. But now I'm confused why there are 2 definitions in different namespaces that are not in the same order. Can we just use the Mavsdk::ComponentType?

@dakejahl
Copy link
Contributor Author

dakejahl commented Oct 4, 2024

This is hard to do without introducing circular dependencies. Can we have some kind of global_include.hpp where we can define things common to the core classes?

Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thanks for this.

I absolutely don't want a global_include.h file, however, I also don't want circular dependencies, so we'll have to clean it up.

I also don't want multiple component enums. I thought I had already cleaned that up but it looks like I haven't, so I should do that.

src/mavsdk/core/include/mavsdk/mavsdk.h Outdated Show resolved Hide resolved
@julianoes
Copy link
Collaborator

#2416

@dakejahl
Copy link
Contributor Author

dakejahl commented Oct 4, 2024

Rebased off of your branch

…mbal in places, consistent ordering of enums
Copy link

sonarcloud bot commented Oct 7, 2024

Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning my changes up and adding yours :)

@julianoes julianoes merged commit ca0c41e into mavlink:main Oct 8, 2024
36 checks passed
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