Decode of the Toggle command doesn't look right

[HakanL]

I checked the decode function and it seems that it will put both Level and ToggleCount from the same part of the string, and also the check for ToggleRate seems wrong. I don't have a way to actually test it, but I was create a C# implementation of the UPB spec, and it looked inconsistent.

[DaAwesomeP]

You found a bug alright. Thank you.

I order to make the decode function, I copied & pasted the generate function and reverse engineered it. I never quite fixed that if statement it seems. I'll fix it as soon as possible.

[DaAwesomeP]

I also never made a way to distinguish between level and toggleCount. My bad.

[DaAwesomeP]

Actually, now that I look back at the UPB PDF, toggle cannot accept a level setting at all. Patches have been released patches for upb and upb-cli. Thank you again for spotting this.

[DaAwesomeP]

BTW, you can test it with upb-cli. Simply generate a command and then decode it. It should have the same output to the input. If you need help working with that please post an issue there.

[HakanL]

Nice, thank you! Btw, do you have a link to that UPB PDF?

[DaAwesomeP]

At the bottom of the README there is a link called "Tech Specs." That has the UPB protocol PDF, the PIM PDF, and the command wizard.

Also, when you are done with the C# implementation and if decide the to open source it, it would be nice if you could drop a link to it here. I would like to watch it, as your bugs and UPB protocol inconsistencies (like the one you brought up) could apply to my code too.

Also note that your work will be a derivative of mine. I am using the Apache license, which has a few minor rules about attribution and changes. See the redistribution section from here: http://www.apache.org/licenses/LICENSE-2.0

Good luck! It's nice to know that someone is using my work.

On Jan 13, 2015, at 10:46 PM, "Hakan Lindestaf" notifications@github.com wrote:

Nice, thank you! Btw, do you have a link to that UPB PDF?

—
Reply to this email directly or view it on GitHub.

[HakanL]

Ah, thanks..

That's fine, but I haven't copied your code, I was just looking at it to learn how you interpreted different parts of the protocol (like the variable toggle parameters). I guess I misspoke about "porting it", I meant I'm writing a C# port for the UPB protocol.

[HakanL]

FYI here's my implementation of the UPB protocol in C#: https://github.com/iostorm/iostorm/blob/master/CorePlugins/UpbPim.cs

[DaAwesomeP]

Cool! You should remove line 95. That was the error uncovered in this issue. Level cannot be used with the toggle command.

[HakanL]

Thanks for the feedback, I left that part as-is in my code because I wanted to see if I had misinterpreted the specification compared to your implementation. A recommendation on your code, you have byte1-4 and then "fullbytes". The byte 1-4 are actually "half bytes", called nibbles. You may want to update your code to make it more clear. I also recommend bit-wise operations instead of the large if-statements in your decode function, makes for more robust code.