Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Wolfvision eye14 drivers PPT-368 #112

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

polonski
Copy link
Contributor

@polonski polonski commented Mar 2, 2021

  • Wolfvision eye14 drivers rebuilt in Crystal.

@polonski polonski requested review from tob1k and websymphony March 2, 2021 02:05
@polonski polonski changed the title Wolfvision eve14 drivers Wolfvision eye14 drivers Mar 2, 2021
@jeremyw24 jeremyw24 requested a review from stakach March 29, 2021 01:46
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.

looks good
please take a look at the requested change and we can merge


COMMANDS = {
power_on: "\\x01\\x30\\x01\\x01",
power_off: "\\x01\\x30\\x01\\x00",
Copy link
Member

Choose a reason for hiding this comment

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

these commands should be binary, i.e. "\x01\x30\x01\x00"
however that might cause issues in crystal as all strings are UTF8 whereas the ruby code is ASCII (no escape characters) - Ruby also uses binary strings for most IO, whereas crystal has Bytes which might be more appropriate in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

binary commands implemented as suggested. Conversion to Bytes is occurring as expected.

@jeremyw24 jeremyw24 assigned stakach and djmci and unassigned stakach Apr 12, 2021
@jeremyw24 jeremyw24 assigned stakach and unassigned djmci Apr 20, 2021
@stakach stakach self-requested a review April 27, 2021 20:54
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.

I think using the Ruby Driver here as the basis of this driver was a mistake, from the look of it the Ruby driver probably never worked and was based on guess work related to the linked protocol description.

After a quick google search I found Wolfvision has since released a much more detailed protocol description:
https://www.wolfvision.com/wolf/protocol_command_wolfvision/protocol_command.htm#t=instructions%2Fpacket_structure.htm

This indicates that we should probably be starting from scratch with this driver based on that protocol breakdown.

I would also recommend using https://github.com/spider-gazelle/bindata
for describing this binary structure

something like

class WolfPacket < BinData
  endian big
  
  bit_field do
    bool :error, default: false
    bits 3, :head_reserved
    bool :two_byte_cmd, default: false
    bool :two_byte_length, default: false
    bool :two_byte_header, default: false
    bool :set_cmd, default: false
  end

  bit_field onlyif: ->{ two_byte_header } do
    bits 7, :ext_head_reserved
    bool :four_byte_length, default: false
  end

  uint8 :short_cmd, onlyif: ->{ !two_byte_cmd }
  uint16 :long_cmd, onlyif: ->{ two_byte_cmd }

  def command : UInt16
    two_byte_cmd ? long_cmd : short_cmd.to_u16
  end

  def command=(number : Int)
    if number > UInt8::MAX
      self.long_cmd = number.to_u16
      self.two_byte_cmd = true
    else
      self.short_cmd = number.to_u8
      self.two_byte_cmd = false
    end
    number
  end

  uint8 :byte_len, onlyif: ->{ !four_byte_length && !two_byte_length }
  uint16 :short_len, onlyif: ->{ !four_byte_length && two_byte_length }
  uint32 :long_len, onlyif: ->{ four_byte_length }

  def length : UInt32
    four_byte_length ? long_len : (two_byte_length ? short_len.to_u32 : byte_len.to_u32)
  end

  def length=(number : Int)
    if number > UInt16::MAX
      self.four_byte_length = number.to_u32
      self.two_byte_header = true
      self.two_byte_length = false
      self.four_byte_length = true
    elsif number > UInt8::MAX
      self.short_len = number.to_u16
      self.two_byte_header = false
      self.two_byte_length = true
      self.four_byte_length = false
    else
      self.byte_len = number.to_u8
      self.two_byte_header = false
      self.two_byte_length = false
      self.four_byte_length = false
    end
    number
  end

  bytes :payload, length: ->{ length }, onlyif: ->{ !error }
end

@caspiano caspiano changed the title Wolfvision eye14 drivers feat: add Wolfvision eye14 drivers May 31, 2022
@jeremyw24 jeremyw24 changed the title feat: add Wolfvision eye14 drivers feat: add Wolfvision eye14 drivers PPT-368 Mar 26, 2023
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.

4 participants