|
|
|
![]() |
|
|||||||
|
||||||||
![]() |
| Thread Tools | Rate Thread | Display Modes |
|
#31
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
![]() |
|
#32
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
I believe one of the issues with this design is how closely it reflects the JAVA language version, where JAVA does not need to manage memory... hence the red arrows in the diagram (something I never used in my designs) show that objects are not being created and destroyed in the same place. This made it somewhat more difficult to track down the memory leaks. I think the redesign should be c++ based design which does abide by c++ conventions and have clean objects that manage memory properly... and then port this to JAVA... going in that direction... JAVA can simply ignore all calls to deletes, or interface them to do nothing... etc. Anyhow I just wanted to point this out for anyone else who is code reviewing. |
|
#33
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
|
|
#34
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
|
|
#35
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
Code:
write error: : read error: : S_errno_EPIPE S_errno_ETIMEDOUT [[NNTT]] IIOOEExxcceeppttiioonn mmeessssaaggee:: CEorurlodr noont FwDrIiOt er eaaldl [[bNyTt]e s0 xt2o0 ef8dd 1s8t reenatme r[eNdT ]c o0nxn2e8cet8ido1n8 setnatteer:e dS EcRoVnEnRe_cEtRiRoOnR state: SERVER_ERROR [NT] Close: 0x28e8d18 |
|
#36
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
|
|
#37
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
I want to expand a bit on this... one thing I have realized later in life is that time gets shorter and it seems like code is never done. There are so many things I'd have liked to have done in code, but can't find the time to do them. So as we sharpen our skills in coding we have less time to do it (especially if you have kids). So with this kind of perspective I've learned that we try to bring the most functionality forward even though it may not be perfect. When I was younger... I'd love to re-invent the wheel as I could learn from it, but then at some point I learned how to read other people's code and figure out what they intended to do. This is perhaps the most important skill for a software engineer as there is so much existing code out there... it is important as this is needed to be able to work with a team of engineers, and deal with any imperfections, or coding style differences... when all is said and done... it will be just a few lines of code here and there... corrected, and this Network Tables will become a reliable solution. Grant it, there is some code out there that cannot be salvaged, but this code doesn't fall into that category as I think it was well thought out... in spite of the nit-picky things I mentioned previously. I am a pragmatic person who works with imperfect code all the time... we get it done enough to get the job done... if it works... and works reliably then we win... without re-inventing and losing time and going through another cycle of bugs to fix.
Last edited by JamesTerm : 10-03-2014 at 11:24. |
|
#38
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
Additionally, there is a *lot* a 'bad smelling code' -- stupid things like casting pointers to references and back again (which is related to the author wanting to use 'new' for everything -- which wasn't always necessary), using a union to hold multiple types without any reliable way to determine what is in the union, one byte read/writes to sockets … and this is just the beginning. Because of these (and other things), most of the code should be scrapped. It will be easier to rewrite the code correctly than to try and fix its problems. |
|
#39
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
Given this status, whether or not it is easier does not matter to me... what matters is how long will it take. For those who have time to rewrite that is wonderful... but as for me, I have other code to write! ![]() There is just one minor issue left I want to get done hopefully before Dallas, but this issue should be invisible to users even in its current state. All of your other points are valid, but until someone takes the time to do a rewrite... this is all we got, so I'm going to make it work for our team, and anyone else who wants to use it. (Up to this point... it was your goal as well). Once it is proven to be reliable... any new rewrite will introduce new potential risk, and unfortunately it is difficult to find people who have time to test this properly... otherwise we'd have fixed it before now. There is value in code that has been well-tested in spite of its imperfections... if it is proven reliable... that's really all that any user cares about. Have you seen my changes? The author had the same idea on this fix as well. |
|
#40
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Sure, it does work, and our team is planning on using it in its current state. However, I don't intend to rewrite it either, I also don't have the time at the moment.
![]() NetworkTables is a really useful idea, and so I think it would be worth it for the maintainer of the code (eg, FIRST/WPI) to make the code better -- there's a lot of ways it could be improved. However, if they're going to make improvements in the future, then my recommendation is to rewrite it -- and create some unit tests/etc. If done properly, it will make it a lot easier to fix any such bugs in the future. |
|
#41
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
It should be noted that I've been using a win32 version of network tables port quite a bit... I've found it even useful in NewTek development to watch variables that dynamically change. That said... it has never once locked up or crashed... so this is partly why I could never find the issue... I can't reproduce it in win32 environment. I believe the last issue remaining deals with the time it takes to connect to the time it takes to make the initial first write. I'm thinking of putting a sleep in there as well as taking a closer look at ConnectionMonitorThread::run()... I suspect this thread may not be sleeping in some cases... but I could be wrong. |
|
#42
|
|||
|
|||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Hello,
I am joing this converstation late. We just competed in our first district event this past weekend and notices some strange anomolies which maybe contributed to the problem identified here. We use smartdashboard to send booleans back to a dashboard to let the drivers know certain events or robot states have been reached. A few times during pit testing we noticed that we could not successfully command the robot. The robot was in a hung state. Comms were up, but the dashboard was frozen, and while a button press did register on the default dash, no response was displayed by the robot. Restarting the Robot, AND restarting the driverstation seemed to be the only way to get arround this and re-establish comms. I was just writing to make sure I understand the bug because we use Java and it was unclear if this problem was just for C++, on the client side/ ther server side, both... Based on what I read it appears the crux of the problem is that although NT is multithreaded, it holds on to a lock during a write sequence. A write sequence which also blocks and keeps the lock if the write fails. The robot thread uses this same lock to push data, thus causing the hang on the robot. Is this all correct? My question is the code to obtain the lock on the robotside in NT or in SmartDash? If you were to call the smartDashboard putXXX methods in a separate thread, would that not at least prevent the hang? I have not spent any time looking into the NT/SD implemenation, yet, but I will. Again we are using Java on the robot, and the pyNetorkTables port provided by Dustin on the driverstation. Regards, Kevin Last edited by NotInControl : 10-03-2014 at 16:19. |
|
#43
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
Quote:
|
|
#44
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
Quote:
I believe the problem is most severe on C++, but that similar problems may affect the java side, as others have reported less severe problems in Java. I've only glanced at the java version recently, and a cursory look shows the java code does not hold the lock the same way that the C++ code, but my expectation is that there is a similar bug somewhere, given that it's written by the same author. Quote:
Quote:
|
|
#45
|
||||
|
||||
|
Re: Serious bug identified in SmartDashboard/NetworkTables -- robot hangs
It is worth noting that I've never actually tried to get the client to hang -- but I think they use the same code to write to the socket, so I would expect the problem to exist there.
|
![]() |
| Thread Tools | |
| Display Modes | Rate This Thread |
|
|