On Tue, Nov 29, 2016 at 15:59 -0000, Karl Palsson wrote: > > As discussed on IRC a little recently, and in issue: > http://sigrok.org/bugzilla/show_bug.cgi?id=868 > > First, please consider pulling into sigrok-cli support for > multiple protocol decoders and dropping the -S option [1] By > updating sigrok-cli first, it gains proper support for stacking > decoders based on the IDs returned by libsigrokdecode, instead of > it having implicit knowledge of the ids. > > Branch at: > https://github.com/karlp/sigrok-cli/tree/pulls/multipro Commits: > + 0b9982b drop -S stack option > + f999f28 Support multiple protocol decoder stacks > + d9de37a include decoder instance id in normal outputs [2] > + 27105be valgrind: Clear more unfreed memory issues > > [ ... ] > > [2] This changes the CLI output to include the decoder instance > id. Again, how critically important is the CLI output? I would > presume that it's not intended for scraping anyway, the decoders > and output files are for those purposes.
Does it change the output for the "regular" (currently only supported) use with a single decoder session? Can it be made identical to previous behaviour, and only prefix the output with the decoder's ID when multiple decoder sessions are in use? Why am I thinking of grep(1) and -H/-h? Users might want to pick the format they will receive. I would not bet that nobody processes sigrok-cli output in programmatic ways. After all it's a command line tool made for piped operation. And users can specify in detail which annotation of which decoder they'd like to receive, having the output change its format might be unexpected. There's another commit which makes -A apply to all decoders of the specified type, instead of their instance. I'd question this one, and would suggest that the decoder instance ID is used for -A associations. For the regular case of a single stack with multiple (different) decoders, the instance ID is the decoder name. For cases where multiple decoders of the same type are in a decoder stack, or multiple stacks contain the same type(s) of decoder, you'd very probably want to address the annotations per instance, not per decoder type. Users can specify the decoder instance's ID with the -P option, or might as well foretell the instance ID since its construction is "intuitive". Depending on the implementation it's either unique per decoder stack, or the complete set of stacks. > Subsequently, Please consider pulling into libsigrokdecode > support for multiple instances of a decoder in a session. Branch > available at: > https://github.com/karlp/libsigrokdecode/tree/pulls/inst-ids > Commits: > + 8dd7fde valgrind: safely iterate lists > + fe20e5e valgrind: free channels > + 3b722ff support adding multiple instances of a decoder > + fc841fb look up instances by id in the full stack Maybe invert the order? Make the find routine search the whole stack first, and use that feature in the unique ID creation afterwards? Split the creation of the unique ID and the cleanup of releasing resources maybe? Could improve bisectability and maintenance work. Would give proper credit to Soeren if you used his patch and then built on top of it. Reduces merge conflicts, too. In case it's not obvious: I welcome the support for multiple protocol stacks in the CLI frontend. It just should be done in least intrusive ways for users and external processors. virtually yours Gerhard Sittig -- If you don't understand or are scared by any of the above ask your parents or an adult to help you. ------------------------------------------------------------------------------ _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
