search

vwout / obs-visca-control

OBS plugin to control Visca-over-IP based cameras
https://obsproject.com/forum/resources/control-visca-over-ip-based-cameras.1173/
GNU General Public License v3.0
35 stars 6 forks source link

further restructure libvisca classes #9

Closed CapsAdmin closed 1 year ago

CapsAdmin commented 1 year ago

make objects of PayloadCommand and PayloadReply separate objects in do end blocks

I'm not sure how to test this without having a camera, maybe you can do the rest if you think this change is alright.

vwout commented 1 year ago

Thanks for the works! It did cross my mind a few times to also convert these into proper objects, but since these are purely internal to Visca.Message, I did not feel like spending a lot of time on it. Only thing is that I don't like the do ... end scoping style; since all names are proper, no variables are leaked or seem to cause conflicts when left out.

Anyway, all my unit tests still pass (with minor modification), so I will test this on real cameras soon. (Yes, setting up CI on github with unit testing and such is another todo)

CapsAdmin commented 1 year ago

Thanks for the works! It did cross my mind a few times to also convert these into proper objects, but since these are purely internal to Visca.Message, I did not feel like spending a lot of time on it. Only thing is that I don't like the do ... end scoping style; since all names are proper, no variables are leaked or seem to cause conflicts when left out.

Alright, I've reverted the do end blocks.

My main reason for do end blocks is mostly for being able to collapse them in editor. I also find it useful to more easily separate them into files later on if necessary. It makes more sense when you have locals that only need to be accessed by the object itself.

Anyway, all my unit tests still pass (with minor modification), so I will test this on real cameras soon. (Yes, setting up CI on github with unit testing and such is another todo)

Are the unit tests somewhere else public right now?