Skip to content
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

Opinion: assigning the medium of an entity in an actuator is a bug #66

Open
allsey87 opened this issue Mar 30, 2018 · 2 comments
Open

Comments

@allsey87
Copy link
Collaborator

Referring to this line of code, I think the adding of LEDs to a medium here has been done as a work around so that entities like the foot-bot etc don't need to declare which medium their LEDs should be added to in the XML. This solution, however, just feels like a bit of a hack to me and possibly will lead to confusion and bugs in the future. For example, consider the case where I set the LED medium of the LEDs on an entity and then override that with the medium specified by an actuator.

It doesn't make any logical sense for an actuator (considering what it is and its functionality) to change such a property of an entity. If we want to keep the XML minimal, I would suggest adding a single attribute to the foot-bot and related entities as follows:

<foot-bot id="fb" led_medium="leds" rab_medium="rab">
   <body position="0,0,0" orientation="0,0,0"/>
   <controller config="x"/>
</foot-bot>

Or alternatively albeit more verbose:

<foot-bot id="fb">
   <body position="0,0,0" orientation="0,0,0"/>
   <leds medium="leds"/>
   <rab medium="rab"/>
   <controller config="x"/>
</foot-bot>
@ilpincy
Copy link
Owner

ilpincy commented Mar 30, 2018

I see your point.

The solution currently in place is, in fact, not a hack. It's meant to prevent media from adding entities that are not enabled (e.g., if you have an LED medium, but some robots do not use LEDs while others do). When you have simulations with hundreds of robots, this kind of feature proves critical for performance and memory efficiency.

Thus, the idea is that the act of adding an actuator or sensor makes it worth adding the entity to the medium. Unfortunately, this can't be checked at entity creation, so it's the actuator's job to do this.

If we follow your proposition, we could still have entities added to media conditionally. However, the critical issue becomes memory efficiency - for every single entity, we need to store the id of every potential medium an entity might be added to. When we want to simulate hundreds of robots, this is a huge waste of memory.

Also, from a design point of view it makes sense for an actuator/sensor to manage the medium. ARGoS allows for multiple versions of actuators and sensors, and media are meant to help simulate those. Improved actuators/sensors with different types of media (or no medium at all) would not be as easy to achieve, or as usable.

@allsey87
Copy link
Collaborator Author

From my understanding of the new logic for enabling/disabling that you added in December (fffd3e5), it seems like we could just do the following in the init code for the LED actuator:

if(! m_pcLEDEquippedEntity->IsEnabled()) {
   m_pcLEDEquippedEntity->Enable();
}

And this would trigger adding the contained LED entities to the medium (if they were not already added, e.g., in the case of the box entity) and I don't think there would be any difference in performance. The bit I don't like is how the medium is configured in the actuators, enabling seems ok to me. To make this clean and consistent, however, I think there should be a flag in the actuators such as m_bEntityEnabledBeforeInit or something to that extent which we use when destroying the actuator (i.e. if this flag is true then don't disable the entity when destroying the actuator).

Just food for thought, but I think things like this are important if we want to avoid headaches in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants