-
Notifications
You must be signed in to change notification settings - Fork 140
Remove deprecated TraCI API and switch to libsumo/libtraci #390
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
base: master
Are you sure you want to change the base?
Conversation
riebl
left a comment
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 think the tools/testing stuff should be a separate PR because it is not directly related to the libtraci switch.
| # to the existing SUMO instance is only possible with slower libtraci API. | ||
| # Switching both options to OFF disables SUMO integration. | ||
| option(WITH_LIBTRACI "Build Artery with SUMO support via libtraci" ON) | ||
| option(WITH_LIBSUMO "Build Artery with SUMO support via libsumo" OFF) |
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.
No libsumo for the moment, right?
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.
Only one of those should be set at once :)
291bf6f to
9361351
Compare
|
Hey, @riebl! I wanted to discuss the cpp looks a little. I know you don't like identing namespaces, but how about base classes? I'm used to this class A
: public B,
, public C {
/* contents */
};Which currently formats as class A : public B, public C {
};The first one looks cleaner I guess |
|
Also, I've looked into variable cache - it seems still useful (even with libsumo), but I've switched to more template-heavy implementation. Can you have a look? Keep in mind, I have not even compiled it yet, so I wish you could just give your overall impression. |
This is an initial PR for requested and awaited switch. There's still much more work to do here, I'll post it so others may track progress.