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

Topics /paramset/ not working #23

Open
dersimn opened this issue Nov 6, 2017 · 4 comments
Open

Topics /paramset/ not working #23

dersimn opened this issue Nov 6, 2017 · 4 comments

Comments

@dersimn
Copy link
Contributor

dersimn commented Nov 6, 2017

In line 140, you're calling the same function for /set/ and /paramset/, however the corresponding implementation always uses the RPC method setValue in line 361. For paramsets other than VALUES you have to use putParamset instead.

You could use putParamset for everything, because it also supports VALUES, but you have to encapsulate the data into an additional object, like here. I'm not sure how your solution about explicitDoubles will work with that, but you'll get the idea.

@hobbyquaker
Copy link
Owner

Oh yes, you're right. Obviously never tested that. I would prefer to stick with setValue for /set/ topics - as this is the way eQ-3's documentation describes for setting distinct values in the VALUE paramsets. Regarding the explicitDouble: we should iterate through a putParamset payload and call rpcType() for every value contained in the payload to make sure that the correct types are sent. Will try to implement it next weekend, but would also appreciate it when you want to do it :-)
"Lessons learned" from the /paramset/ thing: 46% test coverage is by far not enough ;-) Will try to implement more tests asap, but as before - help is highly appreciated :-)

@dersimn
Copy link
Contributor Author

dersimn commented Nov 7, 2017

I'll do, but probably not before next week as well. I'm still in the process of getting familiar with the XML-RPC stuff, but I discovered a quite funny thing in my "destroy everything"-branch, here's the diff, and here the gist for testing:

It seems that rfd is now accepting <string>22</string> messages and auto-converts it to float (tested with fw 2.29.22 in YAHM and 2.29.23 on a CCU2).
This might not be very protocol conform (there isn't much of a protocoll description about the details anyway), but it works perfectly. Even ENUM type variables can now be set by sending the corresponding strings, the e.g. in my gist sets the

"BOOST_TIME_PERIOD": {
  "VALUE": 6,
  "DEFAULT": 1,
  "FLAGS": {
    "VISIBLE": true,
    "INTERNAL": false,
    "TRANSFORM": false,
    "SERVICE": false,
    "STICKY": false
  },
  "ID": "BOOST_TIME_PERIOD",
  "MAX": 6,
  "MIN": 0,
  "OPERATIONS": {
    "READ": true,
    "WRITE": true,
    "EVENT": false
  },
  "TAB_ORDER": 21,
  "TYPE": "ENUM",
  "UNIT": "",
  "VALUE_LIST": [
    "0 min",
    "5 min",
    "10 min",
    "15 min",
    "20 min",
    "25 min",
    "30 min"
  ]
}

of an HM-TC-IT-WM-W-EU. Your current implementation uses a quite complicated way for this conversion.

@hobbyquaker
Copy link
Owner

hobbyquaker commented Nov 7, 2017

Hi Simon, for clarification: the purpose of

        case 'ENUM':
            if (typeof val === 'string') {
                if (paramset.ENUM && (paramset.ENUM.indexOf(val) !== -1)) {
                    val = paramset.ENUM.indexOf(val);
                }
            }

in rpcType() is to set enum values by their string representation, so e.g. MANU-MODE instead of the integer index 0. But I'm not sure if this is useful, never used it until now :)

@dersimn
Copy link
Contributor Author

dersimn commented Dec 1, 2017

It works the same way when you just send MANU-MODE as a string to the CCU ✌️

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