XML config is causing more harm than good

[SteveKing]

All right, after hours of debugging I found the problem: The XML config file was renamed from "XML Bot Config.xml" to "Bot_Config.xml". The robot had one of each, with different contents. The devel branch uses one, the stable tags use the other. Much confusion and long hours of debugging ensued!

Okay, I call it a failed experiment. It's a noble idea, but the config file is too tightly coupled to the code. We can't really change one without changing the other as well. It would serve us much better to have the control mappings compiled into the binary so we always know they match each other.

Tomorrow I'm going to change the code so the config is compiled in. It may still be XML; I might end up just wrapping the whole file as a string and passing that to the XML parser. We can't keep losing time to a mismatch like this.

In the meantime, I have a tag with working drivebase and lift pneumatics. (I haven't tested the arms.) It relies on the Bot_Config.xml file in the src directory, which is currently installed on the bot.

[C7C8]

I'd like to make note of the fact that XML config file name change change was documented in commit eccf60a (https://github.com/Dreadbot/Dreadbot-V/commit/eccf60a348bc4a43b4b780f2beda240c05b64098), and I did make at least Parker aware of the change at the time. At the time it was changed, I don't think you were there, so I was unable to inform you. As detailed in the commit message, it was actually changed to simplify the name and removed the space, which was requested some time ago (I just never got around to changing it). Both files remained on the robot - as I discussed with Parker at the time - to maintain compatibility so a robot running code from another branch would not immediately crash (although there is a handy smartdashboard put that outputs any and all output from the XML parser as a string).

As for the control mappings, those ARE written directly into the code - check out Robot.cpp. Every last control (except drivebase stuff; more on that later) is hard coded as a number with a small comment off to the side to indicate what control the ID corresponds to. The "load control mappings" from XML was discarded some time ago in favor of retaining programmatical control over motors and pneumatics to allow for more advanced control options.

At its core, the XML config idea is designed around the concept of having rapidly modifiable controls and having the ability to support two different robots with different hardware configurations. To date, that idea still works, and we have been using it for a while - just last week I ended up making a radical change to the robot and how controls are changed by changing two lines of XML code, no compile/deploy to the robot needed. XML is still working fine, as far as I can tell - this "change the code, change the config" problem is present in virtually all systems that use a config file of any kind. Yes, it's something that needs to be worked around, but it's not completely unworkable, and the benefits of using config files over hardcoding values in are often enormous. All I see this particular instance as is a failure in communication, not in code; in the future, I will make sure to send out an email (or something) any time there is a major change such as what recently occurred.

The only controls currently configured to be run directly though the XMLInput class are drivebase controls, at the moment - XMLInput was largely stripped of its control-handling abilities many weeks ago. They currently remain configurable through XMLInput because they are frequently subject to change, and, in all honesty, having a load of drivebase stuff (inverts, deadzones, rampups, etc) floating around in Robot.cpp just makes for messy code. Besides, being able to easily modify drivebase controls may come in handy at competitions (not to mention the ability to easily modify any hardware configuration at any time) - it's far easier to do a quick file swap over FTP than it is to do a full compile and deploy.

[C7C8]

I just wrote a (very) small batch script for transferring the XML file to the robot that should run on any WIndows computer (sorry, I don't know how to write scripts for Linux). It's currently located on the XML_Deploy branch (I didn't want to push it to develop in case it doesn't work as I intended).

It automatically connects, deletes the old file, replaces it with whatever XML file you have in your src directory, and closes the connection. This should largely automate updating the XML file, so when deploying code to the robot using Eclipse, I highly recommend that we all get into the habit of running this script whenever we do a deploy to the robot. It's not a perfect way to eliminate the differing-XML-files issue, but it's significantly better than using an FTP client like FileZilla every time we want to update the XML file.

[SteveKing]

This isn't about saying that the XML input was a bad idea, or even that it's a bad implementation. I liked the idea of configurable controls and motors, and this solution worked well. But it turns out to solve a problem we don't really have. Motor and solenoid assignments are pretty much cast in stone at this point. They're not going to change, so having them in a separate config file doesn't buy us anything. The controller assignments are still in flux so they might be appropriate to have configurable, but we're not using the config file for them. And, when the XML fails it doesn't do so in a way that's very obvious.

The fact of the matter is that Parker and I spent two and half hours debugging code and neither one of us thought to examine the XML in detail. It was there and it looked okay, but it didn't match the code and we didn't notice. When I did get to the point where I discovered the name change it didn't help that Internet Explorer lied to me about the contents of the directory. It silently showed me a cached copy of the directory listing instead of the current one. The cached copy didn't list Bot_Config.xml, so I spent another two hours trying to figure out how the heck the devel branch worked at all without its XML file present. We lost a day of driving and programming time, and at this point the team just plain can't afford that.

I'm not trying to assign blame. Maybe it's a problem with the tools. Maybe it's just a weakness in the way I debug things. I'll cop to that. It doesn't matter what the reason is. The three of us are the only programmers we have. If two of us together can't diagnose and fix something in a reasonable amount of time it doesn't matter how good the idea or the code is, it's not working for us. We need to replace it with something less flexible but less prone to operator error, and move on to the important auton code.

[Fraktality]

Chris, I have to agree with Mr. King. XMLConfig was a useful tool during the build season for performing tests, but the I/O is now explicitly defined. Further modification of the pin mapping is unlikely and even undesirable; especially during runtime. An option we should consider is to define the ports in a header file using the preprocessor.

[C7C8]

I will be willing to change from loading a file to loading from a header, so long as the XML class itself is maintained as well as what it is currently responsible for - it and the modules it uses are currently very deeply engrained into the robot code - it's really not worth changing those, since there's no gain from going to the time to write replacements. The original modules aren't exactly dependent on XML in the first place.

I can start writing a header file immediately, and will switch the load XML file function from loading an actual file to just loading values from the header file (remember, motor/pneumatic groups are still created from loaded config). It shouldn't take much time to switch to embedded config over XML config.

Also, I'd like to note that, since there were ever any XML loading files, there's a SmartDashboard::PutString() that directly outputs the result of loading the XML file to the smart dashboard. If anything bad occurs, it outputs a human-readable error message. Usually, it just says "No error", but when I've had issues loading the file, it says "Error: file not found" or something like that. I did do my best to make it fail obviously, it's just that with robot stuff it's difficult to get any really obvious issues shown.

TL;DR - I'll start converting to header-based controls immediately. It won't take long, so don't worry about it.

[SteveKing]

On 2015-02-24 14:01 , Sourec wrote:

TL;DR - I'll start converting to header-based controls immediately. It won't take long, so don't worry about it.

Chris, I'd really prefer you spent your time on the auton code. I can do the I/O changes. And yes, I agree that the existing logic should be kept, just substituting values from the header for values from the XML file.

Steve King steve@narbat.com mailto:steve@narbat.com

[C7C8]

I switched it over - it was an incredibly quick fix. Also, auton code is done at the moment - I need to test it before I can go any further. I'll be at robotics today (for a while), so I might be able to test.