ilpincy / argos3

A parallel, multi-engine simulator for heterogeneous swarm robotics
http://www.argos-sim.info/
268 stars 121 forks source link

Query documentation cleanup #167

Closed jharwell closed 3 years ago

jharwell commented 3 years ago
<foot-bot>
    ...
    <COMPONENT attr1="value1"
                               attr2="value2"/>
    ...
</footbot>

This eliminated a lot of long attribute names (e.g. "omnidirectional_camera_aperture"), and makes it easier to add new XML attributes to a given component's configuration without worrying about naming clashes, because all components have their own tags now. This also simplifies the Init() calls by initializing all components the same way: first initializing the component with the default values for the robot, then calling Init() to override default values through the XML.

Additional XML parameters were documented:

Also, I tweaked the perspective camera configuration to allow the user to specify the camera look direction: up, front, or down. The docs now say which configurations are valid for a given robot: foot-bot can look 'up' or 'front', while the eye-bot can only look down, and the spiri can only look 'front' (according to what the code already did for each robot).

ilpincy commented 3 years ago

Should the "anchor" attributes of the CRABEquippedEntity and CPerspectiveCameraEquippedEntity be configurable via XML ? That seems like an ARGoS internal thing that shouldn't be user configurable, but I left it in for now, because it was already there.

It should be an internal parameter, I wouldn't export it.

ilpincy commented 3 years ago

I have checked the code - this is a lot of good work! I need to wait a little before we can make this final, because a few of these changes are breaking (and I am teaching a course based on ARGoS). I think we need to come up with a strategy to ease the transition to this new version. Still, I like these changes, they make the XML file much cleaner.

jharwell commented 3 years ago

I removed the exportation of the anchor attribute.

As far as a transition strategy, I can think of (1) updating the examples to reflect the new configuration definitions, (2) adding some new configuration examples (there is only currently 1 example for the omnidirectional camera), and (3) creating the devel branch we talked about for the travis-ci integration, merge this into it, so that this can be closed and you can merge it into master whenever it seems appropriate.

Thoughts ?

jharwell commented 3 years ago

@ilpincy Can this be merged ?

jharwell commented 3 years ago

@ilpincy @allsey87 I would like to merge this (ideally Monday 10/18). This will break existing XML configuration for affected sensors/actuators, but at discussed above, the changes result in cleaner XML. What needs to happen/what do I need to do to transition things over? The only thing I can think of is update the examples.

ilpincy commented 3 years ago

I appreciate the work, but I prefer to avoid breaking changes. I'm sorry, but I just don't have the bandwidth to handle all the questions / emails I will receive as a result of this kind of change.

jharwell commented 3 years ago

I really hate to see this work making ARGoS easier to use go to waste/be only used by me. I see two potential ways to incorporate this @ilpincy without breaking things for users, which are not necessarily mutually exclusive:

  1. At one point you had talked about an ARGoS 3.1 build as an opportunity to overhaul the visualization system and other things. Wouldn't this be a good thing to include in such a build, where people will know if they upgrade that things might break (i.e., it's a major release which by definition can include breaking changes)?

  2. Create a devel/argos-next/etc branch where we can merge changes that will break things so that they can be tested and debugged, and only released into master at rare intervals (potentially never). That way, people can opt-in to features like this one if they want to, and then know they have to change things, while leaving everyone else unaffected.

If you still don't want to merge this into the ARGoS repo in any way that's OK. I just felt like I needed to push for it a bit.