Skip to content

Conversation

rdiazx
Copy link

@rdiazx rdiazx commented Apr 6, 2022

Description of the change

Ported existing ruby driver code to crystal.

Note - TODO

I didn't add the spec test for this driver. It depends on a driver with generic_name:Mixer - like the q_sys_control.cr - and I'm not sure of the expected return values.

@rdiazx rdiazx requested review from stakach, w-le and pkheav April 6, 2022 21:57
Copy link
Member

@stakach stakach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@stakach stakach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think we should implement the other common camera interfaces
https://github.com/PlaceOS/drivers/blob/master/drivers/sony/camera/cgi_protocol.cr#L8

This implements a bunch of other interfaces
https://github.com/PlaceOS/driver/blob/master/src/placeos-driver/interface/camera.cr#L6

If there is a function that you can't implement due to limitations of what this camera can do, then just have it raise an error

Comment on lines 1 to 2
# encoding: ASCII-8BIT
# frozen_string_literal: true
Copy link
Contributor

@pkheav pkheav Apr 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to mention this the other day @rdiazx but I don't think these lines from the ruby driver are needed in PlaceOS, can you confirm @stakach ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah not needed, all strings are UTF8 in crystal
That was a ruby hack to get around windows encoding leaking in if people edited files on windows

@caspiano caspiano changed the title Chore/port ruby driver feat: port QSC Q-SYS Camera ruby driver May 25, 2022
@jeremyw24 jeremyw24 assigned chillfox and unassigned rdiazx Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants