-
-
Notifications
You must be signed in to change notification settings - Fork 10
Sim first batch #198
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: 185-model-railway-simulator
Are you sure you want to change the base?
Sim first batch #198
Conversation
| m_socketUDP.open(boost::asio::ip::udp::v4(), ec); | ||
| if(ec) | ||
| assert(false); | ||
|
|
||
| m_socketUDP.set_option(boost::asio::socket_base::reuse_address(true), ec); | ||
| if(ec) | ||
| assert(false); | ||
|
|
||
| m_socketUDP.bind(boost::asio::ip::udp::endpoint(boost::asio::ip::address_v4::any(), defaultPort), ec); | ||
| if(ec) | ||
| { | ||
| std::cout << "UDP cannot bind" << std::endl; | ||
| } | ||
|
|
||
| std::cout << ec.message() << " END" << std::endl << std::flush; |
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 suggest we change the return type from void to std::error_code, the start can return an error if something does wrong :) Then the callee can check it and report a proper error :)
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 need some help on this one...
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.
If something fails return an error (the ec value), if the function succeeds return no error: return {};
In the start function the boost functions will return the error code, we can just return that if it is an error.
// some boost function with an ec param
if(ec)
{
return ec;
}The caller can check if the start() succeeds:
if(auto ec = sim.start())
{
// error
}Traintastic server uses this in the Train::acquire function. This is really powerful as you can also return the reason why it failed.
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.
Ok but the logic is run in the worker thread so returned error would be async.
What should I do?
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.
worker thread should exit on error, the main thread then needs to check if the simulator is started or has an error. Those variables are accessed by two threads, so best is to use std::atomic<>:
std::atomic<bool> m_started = false;
std::atomic<std::error_code> m_error;When start() set m_started to true else set m_error.
The main thread then can check (m_started || m_error), after the startup one of them must be true.
|
|
||
| m_acceptor.listen(5, ec); | ||
|
|
||
| m_socketUDP.open(boost::asio::ip::udp::v4(), ec); |
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'd like to have discovery optional, we can add a bool discoverable = false parameter to the start function for that. In most cases discovery isn't needed I'd expect.
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.
Done. Now we should add console argument to turn it on/off. For now it's still forced on because it needs testing.
Also another argument for localhost only server.
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.
yes, command line options to control those would be great.
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.
Should we use boost to parse command line?
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 I see there is already a parseCommandLine() function in main.cpp
I'll use that!
|
@gfgit OpenGL can be removed, I mainly did it as an experiment, if we even wanna have 3D we can built another view :) |
We receive some garbeage chars at end. Use "starts with" instead of exact comparison. - Send TCP port in big endian. - Change UDP port, cannot be same as TCP port - failed attempt to increase send buffer size
6087ce1 to
0381f46
Compare
0381f46 to
2a9b787
Compare
|
Cool didn't know same port can be used! Thanks for the suggestion.
|
|
@gfgit the handshake you added, is that a ping/keep alive? |
|
Yes, the intent is exactly that!
Because I've setup my circuit simulator to have track circuit in occupied state by default (failsafe) and only free them when it connects to traintastic simulator.
When connection is lost, all track circuit should get occupied suddenly but this happens only if both application runs on same PC.
If simulator is in another PC in local network it still thinks to be connected to a ghost simulator.
Hence handshake every second to be really sure we are connected!
We could add this also to traintastic server if you like.
I'm usure about the actual removal of connecton, maybe there are some cleanups of boost socket which I missed...
|
|
Hi Reinder, what is the status on this?
I think I addressed all the remaining issues except returning std error_code on start.
In the meantime, buffer issue is solved by requesting specific channel at a time.
Also I've implemented square signals which go on right side of track and often have light arrow on top to clarify which track they refer to.
Instead the 2 small lights below are "advancemnt" or "call signal".
They are used when there are issues on apparatus so main signal remains red.
To increase automation I've created three new tracks objects:
1) Spawn train, when activated by a curcuit in my simulator, it will create a semi-random lengnht.
2) Remove train, directional.
Will erase train when it gets touched.
3) Change direction, when touched by an automatic train, this will invert travel direction. This node is less useful, I did it just for fun!
I plan to replace its use case with "normal stop locatio" tag on station platforms for commuter trains.
|
|
@gfgit sorry lost a bit track of this one due to other work, can you have a look at the build issues? Seem to be a few warning that can be resolved easily. |
Hello,
these are a first set of changes rebased on current upstream.
Mainly UDP discovery and JSON comments.
I tried to increase TCP send butter to 8192 but problem is still present and async_write() doesn't seem to report error codes in callback so maybe the issue is elsewhere?
After this, if you agree, I removed completely the use of OpenGL to get thicker lines and to draw other objects like signals.