aseba-community / aseba

Aseba is a set of tools which allow beginners to program robots easily and efficiently. To contact us, please open an issue.
http://aseba.wikidot.com
GNU Lesser General Public License v3.0
48 stars 62 forks source link

Nodes with different names but similar nodeIds are not supported #607

Open aobrodsky opened 7 years ago

aobrodsky commented 7 years ago

Problem: Aseba Studio occasionall creates multiple tabs for same target during reconnects.

This problem only manifests itself when there are multiple targets attached at the same tiime.

Reason: getTabFromName in MainWindow.cpp compares name to target, instead of tab (which is what is being looped over).

Fix: Line 2905 of MainWindow should be (I think):

    if (tab->getName(id) == name)

instead of

   if (target->getName(id) == name)

ttyl Alex

stephanemagnenat commented 7 years ago

Reason: getTabFromName in MainWindow.cpp compares name to target, instead of tab (which is what is being looped over).

No, because id is taken from the tab, and then looked up through the target to retrieve the corresponding name.

Do you have a reproducible test case for this issue?

aobrodsky commented 7 years ago

Hi,

Yes. On my mac, the UI for the latest version of Studio falls asleep, creating a disconnect/reconnect problem that I identified earlier. Since I am working on a simulator with multiple bots, Studio has multiple tabs opened. Every time it reconnects, new tabs are added, (except for the first one). So, it seems like it is not finding the correct tab (even though the IDs match up) and creates new tabs.

My assumption was that target refers to the target of the tab currently in the foreground.

ttyl Alex

stephanemagnenat commented 7 years ago

On my mac, the UI for the latest version of Studio falls asleep, creating a disconnect/reconnect problem that I identified earlier

Isn't it similar to the app nap problem we discussed previously (see issue #563)?

Since I am working on a simulator with multiple bots, Studio has multiple tabs opened. Every time it reconnects, new tabs are added, (except for the first one). So, it seems like it is not finding the correct tab (even though the IDs match up) and creates new tabs.

Do you use an external process to put all robots on the same network? I

My assumption was that target refers to the target of the tab currently in the foreground.

Object target refers to the target network. I agree that this is an unfortunate naming, because in general we talk about a target VM, but it is a historical legacy.

stephanemagnenat commented 7 years ago

Could you please check the id of all robots? My guess is that they are all 1, so Studio is confused. I can change playground, but it will only be available for 1.6. Can you compile master branch?

aobrodsky commented 7 years ago

Hi,

Yes. I think cause of the reconnects is the same problem It seems to have manifested itself again, even though the Info.plist for Studio contains the correct flag to turn off AppNap.

On Feb 7, 2017, at 8:39 AM, Stéphane Magnenat notifications@github.com wrote:

On my mac, the UI for the latest version of Studio falls asleep, creating a disconnect/reconnect problem that I identified earlier

Isn't it similar to the app nap problem we discussed previously (see issue #563)?

Since I am working on a simulator with multiple bots, Studio has multiple tabs opened. Every time it reconnects, new tabs are added, (except for the first one). So, it seems like it is not finding the correct tab (even though the IDs match up) and creates new tabs.

Do you use an external process to put all robots on the same network?

Studio connects to the simulator over tcp/ip, and the simulator runs multiple VMs, which communicate with Studio over the same connection. I.e., Any Aseba messages from studio are sent to each of the VMs, and each of the VMs responds back to Studio over the same connection.

All the simulated robots have distinct IDs.

My assumption was that target refers to the target of the tab currently in the foreground.

Object target refers to the target network. I agree that this is an unfortunate naming, because in general we talk about a target VM, but it is a historical legacy.

Thanks, I’ll eyeball the code further.

ttyl Alex

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'

aobrodsky commented 7 years ago

Hi,

All simulated robots have distinct IDs and that is what is reflected in the variable window of Studio.

ttyl Alex

On Feb 7, 2017, at 8:45 AM, Stéphane Magnenat notifications@github.com wrote:

Could you please check the id of all robots? My guess is that they are all 1, so Studio is confused. I can change playground, but it will only be available for 1.6. Can you compile master branch?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'

aobrodsky commented 7 years ago

Hi,

What happens if the controller names (e.g., thymio-II) are different for each simulated robot, e.g., "thymio-II (1)”, “thymio-II (2)”, etc… Will this create problems? My sinulated robots send their own unique controller names to allow the user to easily distinguish between the tabs in Studio.

ttyl Alex

On Feb 7, 2017, at 8:48 AM, Alex Brodsky abrodsky@cs.dal.ca wrote:

Hi,

All simulated robots have distinct IDs and that is what is reflected in the variable window of Studio.

ttyl Alex

On Feb 7, 2017, at 8:45 AM, Stéphane Magnenat notifications@github.com wrote:

Could you please check the id of all robots? My guess is that they are all 1, so Studio is confused. I can change playground, but it will only be available for 1.6. Can you compile master branch?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'

aobrodsky commented 7 years ago

Hi,

I was barking up the wrong tree on this. The problem of multiple tabs though is repeatable.

I believe I found the problem:

The problem is a slightly corrupted .aesl file, which contains two “node” sections, both with nodeId = 1; When this file is loaded, one of the sections is placed in a “(not available)” Absent tab. During a disconnect/reconnect, this is the tab returned by getAbsentTabFromId() and getAbsentIndexFromId(). But, the reconnect is from a Node corresponding to the other section with nodeId=1, with a different name. The result is that the search for the AbsentTab (in nodeConnected()) fails and more and more tabs are added on each reconnect.

The problem can occur whenever we load an aesl file for one configuration of nodes into Studio connected to a different configuration of nodes.

I would think that the right solution would be to perform the search for the AbsentTab in nodeConnected()) based both on name and nodeId.

ttyl Alex

On Feb 7, 2017, at 8:45 AM, Stéphane Magnenat notifications@github.com wrote:

Could you please check the id of all robots? My guess is that they are all 1, so Studio is confused. I can change playground, but it will only be available for 1.6. Can you compile master branch?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'

stephanemagnenat commented 7 years ago

I would think that the right solution would be to perform the search for the AbsentTab in nodeConnected()) based both on name and nodeId.

That is what MainWindow::getTabFromName(...) does, it finds the AbsentTab with the right name and nodeId, and, if not found, the first AbsentTab with the right name. Optionally, a list of node ids to ignore is considered.

I am curious, how did your AESL file got corrupted?

aobrodsky commented 7 years ago

Hi,

I think using getTabFromName() would solve the problem in nodeConnected().

The “corrupted” aesl file can be created in the following manner.

  1. Create an aesl file for one type of controller (say e-puck)
  2. Open Studio and connect to another controller, say thymio-II Studio will complain, but will open two tabs
    • 1 for the original e-puch (not available)
    • 1 for thymio-II Both may have nodeId=1
  3. Save the file.

Voila. You now have a corrupted file, where you have multiple targets with the same nodeId.

Is this a common problem? Not really. I only noticed it because I am creating a multirobot simulator, which assigns unique controller names to each robot, e.g., "thymio-II (1)” instead of just “thymio-II”.

I am realizing now that this will break some assumptions being made by studio and how it loads aesl files. Hence, I will likely go back to using the same controller name for all simulated robots.

ttyl Alex

On Feb 7, 2017, at 12:13 PM, Stéphane Magnenat notifications@github.com wrote:

I would think that the right solution would be to perform the search for the AbsentTab in nodeConnected()) based both on name and nodeId.

That is what MainWindow::getTabFromName(...) does, it finds the AbsentTab with the right name and nodeId, and, if not found, the first AbsentTab with the right name. Optionally, a list of node ids to ignore is considered.

I am curious, how did your AESL file got corrupted?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.


To put it bluntly, we simply do not know yet what we should be talking about, but that should not worry us, for it just illustrates what was meant by "intangible goods and uncertain rewards". -- Edsger Dijkstra, 'The End of Computer Science'

stephanemagnenat commented 7 years ago

It is true that Studio currently is able to handle different nodes with same names and different nodeIds, but not different names with same nodeIds. Indeed having unique nodeIds within a single network is an assumption of Aseba. That is why the switch has the remap identifier option and the new switch undergoing development will do that remapping automatically.

aobrodsky commented 7 years ago

Hi,

This makes sense. From a UI perspective, it may make sense to include the nodeId in the tab title in Studio, for users with multiple robots. The downside of this is you would not be able to use the tab text to store the robot’s name, i.e., nodeConnected() would break.

ttyl Alex

stephanemagnenat commented 7 years ago

This makes sense. From a UI perspective, it may make sense to include the nodeId in the tab title in Studio, for users with multiple robots.

Indeed, but it is not trivial as you have noticed, because currently this name is used to identify the tab in case of disconnection. So we would need a new parent class of all tab widgets, containing this name, so that the displayed name could have additional information.

For this reason, I am moving this wish out of 1.6 milestone. Pull requests with changes are welcome :-)