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

Ola streaming client reports invalid value. #1198

Open
rewolff opened this issue Feb 7, 2017 · 12 comments
Open

Ola streaming client reports invalid value. #1198

rewolff opened this issue Feb 7, 2017 · 12 comments

Comments

@rewolff
Copy link
Contributor

rewolff commented Feb 7, 2017

When I do:
ola_streaming_client -u 0 -d 2,255,2,255,0
ola_streaming client reports: Invalid value 0
And I must say that I disagree. zero is en entirely valid value.. The statement "invalid value 0" is simply wrong.

Now, it could quite possibly be the case that in some context the value "0" is not valid. But the error message should reflect that.

Of course it is easy for a programmer be writing the code for parsing "universe IDs", and that when the parsing fails, then to report "invalid value". But by the time the program suite is further along, such a failure comes from somewhere in the program, and as a user I'm left to guess WHY it considers "0" an invalid value.

Could it be that I wrote O instead of 0? No. Could it be that "0" is not a valid universe ID? No. (I checked the web interface, and I have universes with ID 0 and 2. Apparently I wrote the script when 0 and 1 were valid universe IDs... ).

P.S. The issue is not getting my ola_streaming_client commandline to work, but to improve the software.

@peternewman
Copy link
Member

Have you been able to confirm which of the two zeros in your command is causing the error?

@rewolff
Copy link
Contributor Author

rewolff commented Feb 7, 2017

tree:~> red
Invalid value 0
Invalid value 1
tree:~> 

The "red" script will

  1. turn an RGB led connected to GPIO red.
  2. use the ola-streaming-client to set my DMX RGB lamp to RED.

It does "2" for both the 0 and the 1 universe, because at one point in time the physical lamp was connected to the other universe... Anyway it seems to be the universe id that is the problem. "1" for a universe id is invalid at the moment. I still don't understand what's wrong with "0".

@peternewman
Copy link
Member

Can we see the script please?

What version of OLA are you using? What OS etc? How did you install it?

What's confusing me, is those errors must be coming from here:
https://github.com/OpenLightingProject/ola/blob/master/common/base/Flags.cpp#L208

To reach that, the flag has to not have_arg() which obviously the universe one should have. Then it has to fail to set the value to "1", which all flag types should support.

TBH this is feeling rather like a preprocessor issue with the macros to me, assuming you compiled it yourself.

@rewolff
Copy link
Contributor Author

rewolff commented Feb 8, 2017

I am using OLA compiled from GIT because i'm also adding a driver for some hardware that I'm building.

It seems I'm using an "own-compiled" version, because which ola_streaming_client returns /usr/local/bin.
The script is:

#!/bin/sh
gpio mode 0 out
gpio mode 2 out
gpio mode 3 out
gpio write 0 1
gpio write 2 0
gpio write 3 0

ola_streaming_client -u 0 -d 255,2,2,255,0
ola_streaming_client -u 1 -d 255,2,2,255,0

Ok Some more experimenting:


tree:~/ola/ola.git> ./examples/.libs/ola_streaming_client -u 1 -d 255,2,2,255,0
Invalid value 1
tree:~/ola/ola.git> ./examples/.libs/ola_streaming_client -u a -d 255,2,2,255,0
Invalid value a
tree:~/ola/ola.git> ./examples/.libs/ola_streaming_client  -d 255,2,2,255,0
Invalid value 255,2,2,255,0
tree:~/ola/ola.git> ./examples/.libs/ola_streaming_client -d 255,2,2,255,0 -u 1
Invalid value 255,2,2,255,0
tree:~/ola/ola.git> ./examples/.libs/ola_streaming_client -d 255,2,2,255,0 -u 0
Invalid value 255,2,2,255,0
tree:~/ola/ola.git> where ola_streaming_client
/usr/local/bin/ola_streaming_client
/usr/bin/ola_streaming_client
tree:~/ola/ola.git> /usr/bin/ola_streaming_client -u 1 -d 255,2,2,255,0
tree:~/ola/ola.git> 

@peternewman
Copy link
Member

Okay, so I see the default shipped version works, so let's try and narrow down why your compile is failing.

Can you try the install version of you're compiled version, just to check it's not the lack of ltmain wrappers breaking it:
/usr/local/bin/ola_streaming_client

Can you try building either from the 0.10 branch or this PR and see if they work:
https://github.com/OpenLightingProject/ola/pull/1199/files

The PR should give us some more clues as to where and how flag parsing is failing for you.

@peternewman
Copy link
Member

Can we also have all the traditional info like OS flavour/version, compiler version etc.

Do any of these commands parse their args correctly?
ola_recorder
ola_uni_stats
ola_rdm_discover
ola_usbpro
ola_latency
ola_artnet
ola_throughput
ola_streaming_client
ola_e131
ola_timecode

@rewolff
Copy link
Contributor Author

rewolff commented Feb 8, 2017

OS: Raspbian GNU/Linux 8
Calling the other programs is more of a hassle than I'd like.... I wouldn't know how to call them. Sorry.

I modified common/base/Flags.cpp to:
cerr << "option " << flag->name() << "refused to activate." << endl;
Your suspicions about library issues is correct. Running the script ./examples/ola_streaming_client, simply works, and ./examples/.libs/ola_streaming_client will come back with the OLD message, not my changed message!

I think my modification of the code is better than yours.

  • It mentions the name of the option.
  • It explains that trying to call the activate function (setting value to one) failed.

@peternewman
Copy link
Member

It would be good to run one or two of the other commands just to rule out an issue with that specific utility. They vast majority of them have man pages, or you can just do ola_rdm_discover -u and ola_uni_stats (doesn't need a -u).

I believe if you don't run the ltmain wrapper (./examples/ola_streaming_client) if will run the code in that program, but find any libraries centrally, which in your case could be either the previously installed local ones in /usr/local/lib or the debian packaged ones in /usr/lib, neither of which will have your change obviously.

Can you just confirm that running ./examples/ola_streaming_client fixes the problem and it sends the data as expected? In which case it was probably a bug in a previous version that we've since fixed, or some quirk of not running it via the ltmain wrapper.

Regarding code modifications, are you comparing your proposed patch to the existing code, or to my PR I linked to before ( https://github.com/OpenLightingProject/ola/pull/1199/files#diff-cb14df65f07890e5bf7f649d05439e5bR208 )? Mine also mentions the name of the flag, personally I'm inclined to steer away from activate and not immediately obvious what it does. It's also potentially confusing with the flag present concept we've got. Essentially a flag with no argument is a special case of a boolean flag, equivalent to saying --flag-name true, without having to write the true bit, hence I'd rather keep the error message similar to what you'd see with a bool flag requiring an argument if you used an invalid argument (e.g. tru), compared to introducing a new term like activate. TBH that particular error basically shouldn't ever happen, so I'm still rather confused how it did, you got a bool flag that didn't accept 1 as a valid value!

@peternewman
Copy link
Member

Okay, I think you have a version of OLA on your machine prior to 0.9.0 which it was loading the library from, we used to only support true/false in a bool, where the flag parsing code passes in 1, t/f and 1/0 were added later here:
91ac32b

Which I think explains it. We could in theory change the 1 the flag code passes in for a true, which would then be backwards compatible, but we've never tested or promised that you can use a newer front end with older libs (in comparison, we try to be careful that the other way round works for API usage), so who knows what else would break still.

@peternewman
Copy link
Member

@rewolff have you had a chance to confirm please?

@rewolff
Copy link
Contributor Author

rewolff commented Mar 5, 2017

Sorry. Have been very busy lately. No time for OLA stuff.

@peternewman
Copy link
Member

Have you had any time since @rewolff ?

@peternewman peternewman modified the milestones: 0.10.4, 0.10.5 May 14, 2017
@peternewman peternewman modified the milestones: 0.10.future, 0.10.5 Jul 5, 2017
@peternewman peternewman modified the milestones: 0.10.future, 0.future Mar 5, 2019
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