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

Using memory converters #78

Closed
cdondrup opened this issue Dec 15, 2016 · 5 comments
Closed

Using memory converters #78

cdondrup opened this issue Dec 15, 2016 · 5 comments

Comments

@cdondrup
Copy link

Hi,

I have a question regarding the memory converters. I found a nice function that currently doesn't appear to be used: https://github.com/ros-naoqi/naoqi_driver/blob/master/src/naoqi_driver.cpp#L1158-L1225 which allows to add arbitrary memory keys using a config file like this:

{
        "memKeys": [
                "Device/SubDeviceList/Battery/Charge/Sensor/Value",
                "Device/SubDeviceList/Battery/Charge/Sensor/TotalVoltage",
                "Device/SubDeviceList/Platform/ILS/Sensor/Value",
                "Device/SubDeviceList/Battery/Charge/Sensor/Charging"
        ],
        "topic": "battery_test",
        "frequency": 10
}

To automatically create a topic that publishes the values of these keys:

$ rostopic echo /naoqi_driver_node/battery_test -n 1
header: 
  seq: 2
  stamp: 
    secs: 1481794426
    nsecs: 279854903
  frame_id: ''
strings: []
ints: 
  - 
    memoryKey: Device/SubDeviceList/Battery/Charge/Sensor/Charging
    data: 4
floats: 
  - 
    memoryKey: Device/SubDeviceList/Battery/Charge/Sensor/Value
    data: 0.949999988079
  - 
    memoryKey: Device/SubDeviceList/Battery/Charge/Sensor/TotalVoltage
    data: 28.329000473
  - 
    memoryKey: Device/SubDeviceList/Platform/ILS/Sensor/Value
    data: 1.0
---

The only confusing thing being that the bool value Device/SubDeviceList/Platform/ILS/Sensor/Value is published as a float.

Since, it didn't work out of the box, wasn't called anywhere, and I had to add functionality to find the config file I created, I assume that this is not used (or ever has been?!). My question is, is there a reason for not using this?

I mean it obviously uses ALMemory.getData quite a few times which is bound to be slower than implementing a specialised converter that queries all your memory keys using only one ALMemory.getListData call but apart from that I don't see a reason.

Also, is there similar hidden functionality for inserting data into the memory? A quick grep for insertData seems to suggest there isn't.

@suryaambrose
Copy link
Member

Hi,

First of all, I am glad you found that functionality. It was meant to avoid us the bothersome to add a piece of code for each possible memory key (there are so many..). If it is not used, it is mainly because the most used memory keys have their own converters / publishers. I am guessing this is the main reason. But maybe the documentation is also not good enough about this function ?

I am however surprised that this function was not working, which functionalities did you need to add ?

About your bool becoming a float, I don't exactly remember the reason for this, but it might be a limitation of ALMemory or libqi. You can see here that the type of the data is checked, but bool is not there. However, your data is properly sent. Which could mean that the bool value is incorrectly returned to us as a float.
BTW, we DO use getListData ^^.

Finally, there is no secret function for inserting data. This bridge's main purpose was to perform read-only operations. Besides, inserting data in ALMemory can be a bit more complicated than simply calling insertData when it is related to sensors (because sensors will keep inserting their own real data, which might cause weird behavior, so sensors must be turned off, and it's a bit more invasive).

@cdondrup
Copy link
Author

Hi, Thanks for the quick reply!

The changes I made are quite minor and can be found here: cdondrup@e332fbc

Mainly, I have to find the config file, call the function, and removed the check if the nodehandle is NULL because that is obsolete I guess and only prevents it from working.

I can, of course, live with the bool being a float but it is a bit unexpected. After seeing that there is no field bools in the message I had my money on int ;) Regarding the getData I was looking at the wrong file, thanks for the clarification! I got confused by this function which is also never used.

I understand that inserting data is a bit more complicated that's why I hoped someone else might have done that already.

@suryaambrose
Copy link
Member

The truth is we already did it from time to time (inserting data) but the hack we needed to do to make it work smoothly are so dependent on the key we want to modify that it never made it out to the open-source world :p

About the function you were looking, it is basically the same you are using, but for only one key.

Finally, I understand what you meant by "never used". It is indeed never used in the code. But actually, we use them.
Those functions are "registered" in libqi here which means they are actually part of the public API. But AFAIK, it cannot be accessed directly if not compiled and launched in a certain way (the beauty of making a program which bridges two different frameworks, some things work in one and not in the other).
Basically, once the program is compiled with qibuild and launched with qilaunch, we can do something like
qicli call Bridge.registerMemoryConverter "myMemKey" 10 1 to add on-the-fly a new converter to the bridge for the memory key "myMemKey", which is a float, at 10Hz. This is handy ^^.

I have no idea what it would take to make this accessible to the catkin community though :(

@cdondrup
Copy link
Author

Ah, that explains a lot, thanks for the clarification :)

I just implemented a subscriber that receives the same kind of message that is published by the memory_list converter and adds the values to the memory. Works for basic datatypes and should be fine for our purposes for now: cdondrup@48922f5

If you are interested in any of this (enabling the publisher via the config file and/or the subscriber), let me know and I will open a PR after I did some more testing. If not, feel free to close this issue.

Thank you for your help! Especially the fact that everybody seems to have their own version of the driver makes me feel a bit more relaxed about our own version of the driver ;)

@suryaambrose
Copy link
Member

Great ! Sure you can make a PR once you're done ! :)

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

No branches or pull requests

2 participants