peternewman
commented
3 years ago Hi @Jeff0S ,
Can I ask what mods you've done to ola_recorder? The easiest way to resolve this issue would be to open a PR and get them merged in, then you wouldn't need to deal with conflicts (although you may still wish to test it).
I'd say the same for anyone else who has made local mods, please merge them in, then everyone benefits and you don't have to deal with the maintenance burden alone.
We've had at least two users ask about in/out points and you're the first person to mention not wanting them, so I think there is a desire for them. Whereas you seem to be asking for no changes to be made at all, which is rather untenable.
I think looking at the changes, relatively speaking there will be very few changes to the code path if you're not in simulate in this new version of the recorder. I've also tried hard to ensure that calling it from the CLI it should be backwards compatible. We've also fixed a number of bugs and edge cases with the existing implementation.
In terms of your production code, surely you need to re-test whenever olad itself changes, although we try hard not to introduce new bugs, we only properly test the library parts of the codebase and not the interactions between them, so it's possible for new errors to appear. Aside from bugfixes and support for new protocols I'd wonder if you'd be better off sticking with a fixed release if you don't want to have to re-test.
I think it's perhaps better to think of these examples as examples of how to interact with and test the API, rather than examples of how to programme using it. If you look at ola_client for example it does lots of different things and they're all mixed in together that it's not very easy to find the bits of code you need just to send DMX. In comparison, the Doxygen examples just do one single task in a way you can easily modify for your code: https://github.com/OpenLightingProject/ola/tree/master/doxygen/examples
IMHO, ola_recorder should perhaps never have been in there anyway, and should probably have been moved off separately like ola_trigger as it has a bit more potential than just CLI equivalents of the web interface although it also shows the power of the olad core that a handful of lines of code can give you full playback and recording: https://github.com/OpenLightingProject/ola/commit/cdc569d8165dd8771dda636309f2b0583e4bf6e6#diff-8876d59c8a1e0bf48ba47fc24830c282
You could probably say the same of the rdm_compare and rdm_snapshot python examples.
From the feedback I've got from other people they'd actually like ola_recorder to be far more fully featured for example there is already an OSC interface wrapper ( https://github.com/Lightsteed/OscOlaRecorder ) adding a HTTP one (or a direct OSC one) would be fairly easy and would simplify automation. Some of the commercial recorders allow you to trigger off a DMX channel or universe and again that would be easy and beneficial for people. Having two sets of patching for play and record and a simple way to change between them. Ultimately the code for that is all very simple, the UI and options are as always the challenge.
I think in some ways ola_recorder is perhaps OLAs best kept secret, and if some of that stuff was wrapped up into a nice user-friendly web interface, places like museums could save lots of money on buying some commercial bit of hardware. It could also be a completely standalone project that just uses the OLA API.
Thanks for your feedback though, as you might have seen in the PR we had some debate about backwards compatibility and whether people might be using bits of the code directly, so it's good to get some indirect feedback on this. Also please do let us know if you find any bugs with the changes and we can get them fixed.
In terms of your code, the better thing to do might be to separate it out from the main OLA codebase. Clients don't need to be within the codebase (see for example QLC+) then you could just maintain your fork separately (if you don't want to merge the changes back in), and each time there is a release update olad but keep your code as is, then check for any changes to the ola_recorder code and see if they apply to your version too.
P.S. you probably won't like my other changes to it in #1607 either then sorry (although they are at least more minor).
Jeff0S
For the moment, we stick to 0.10.7 so everything is ok but we had a look at what is going on in the master, and I must say I'm not too fond of the recent changes made to ola_recoder, feature-wise and code-wise...
Not sure adding this start/stop thing will be THAT useful, but hey, why not. However, and it's more annoying, the design changed, code overhead was introduced (normal with such a feature: many corner-cases to handle), etc. => At the end of the day, we cannot keep up-to-date with OLA + cherry-pick our own mods to extend ola_recorder anymore (w/o huge conflicts, I mean) and I guess we won't be the only guys in this case. => Even more annoying (the damn thing being in production), we'd have to re-code/re-test everything for a feature we aren't interested into. => And, then, in the future, the whole thing could yet happen again...
Although they're functional and (VERY) useful, I would tend to think these ola/examples are just made to get coders started with writing OLA clients. I would keep these examples dead simple, or at least, I wouldn't touch them so radically (one can branch/cherry-pick/patch things to extend them just like we do, or contribute some sort of "extended" but -distinct- player if he really wants to). IMHO, these examples should only benefit from bug fixes, or small features (i.e. no impact on the design).
On our end, the "fix" will be quick/easy (either reverting or copying things as they were in 0.10.7), but that would be silly since our ola_recorder would then live "out of project" (e.g. won't benefit from bugfixes anymore, etc).
^ that's why I thought I should ask first: are you sure changing ola_recorder is a good idea ?
Regards, Jeff