-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Added RpcLibPort to change rpc port easily (#1711) #1981
Conversation
@@ -16,6 +16,11 @@ | |||
#include "physics/Environment.hpp" | |||
#include "api/WorldSimApiBase.hpp" | |||
|
|||
#include "api/RpcLibPort.hpp" | |||
|
|||
const extern uint16_t RpcLibPort; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to extern const? https://docs.microsoft.com/en-us/cpp/cpp/extern-cpp?view=vs-2019#extern-linkage-for-const-globals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh.. thank you for the link! I will fix it.
AirLib/include/api/RpcLibPort.hpp
Outdated
|
||
const uint16_t RpcLibPort = 41451; | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of const in a separate file in general, but we don't quite have it in place in the airsim code base.
I think it might make more sense to define rpclibport=41451 in common/Common.hpp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback! I agree with you; separating variables in each file can make code look unclean.
May I change and make a PR again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just saw this now.
Sure, you can also force-push on your current branch, if that's easier.
AirLib/include/api/RpcLibPort.hpp
Outdated
#ifndef air_RpcLibPort_hpp | ||
#define air_RpcLibPort_hpp | ||
|
||
#include "common/Common.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this?
Closing to and opening another PR with compatibility fixes & added settings.json functionality |
I found issue number #1711 while I was trying to change default rpc port,
and thought it might be handy if there is a variable with common value.
Thank you!