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

Audit: Review rcCodes.py & sendKey #122

Open
Ulrond opened this issue Nov 29, 2024 · 2 comments
Open

Audit: Review rcCodes.py & sendKey #122

Ulrond opened this issue Nov 29, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@Ulrond
Copy link
Collaborator

Ulrond commented Nov 29, 2024

Goal:

  • Review the design of rcCodes.py and check whether there's a better approach. is the enum required, and could the translation table be directly used.
  • Re Review the complete list of rcCodes against the olimex documentation codes, which is the valid supported codes for all current removes, every code should be supported.
  • The config map tables -> see https://github.com/rdkcentral/python_raft/wiki/Common-Remote are used to translate the rcCodes.py into the platform specific codes or the codes that the underlying control model will require.
  • rcCodes.py is used as an input to the keyhandler, and that's not clear from the documentation
sendKey(self, keycode:dict, delay:int=1, repeat:int=1, randomRepeat:int=0)

needs to be clear on the types it requires it's not a dictionary it's an enum from rcCodes.py

sendKey(self, keycode:rcCodes, delay:int=1, repeat:int=1, randomRepeat:int=0)
  • review the keyscodes in the list

There are some odd codes in there which are likely removable, unless they're supported by the remote they shouldn't be present.

This would imply that the rc Module was being used for things that it shouldn't be used for, and therefore these are out of scope for usage.

    RESERVE1 = "RESERVE1"
    DCR = "DCR"
    WIFI_SSID = "WIFI_SSID"
    BLK = "BLK"
    WP = "WP"
    LIGHT_SENSOR = "LIGHT_SENSOR"
    USB = "USB"
    RJ45 = "RJ45"
    RS232 = "RS232"
    RESERVE2 = "RESERVE2"

https://github.com/rdkcentral/python_raft/blob/master/framework/core/rcCodes.py

Upgrade rcCode to have a validation function

Upgrading rcCodes to have a validation function something akin to this seems sensible.

from enum import Enum

class rcCode(Enum):
  SUCCESS = "0"
  FILE_NOT_FOUND = "1"
  # ... other codes

def validate_message_code(code_str):
  try:
    rcCode(code_str)  # Try converting to enum
    return True
  except ValueError:
    return False
@Ulrond Ulrond added the enhancement New feature or request label Nov 29, 2024
@Ulrond
Copy link
Collaborator Author

Ulrond commented Nov 29, 2024

Proposed map-table for keySimulator is an example of something new being added, but not populating the rcCodespy

remoteMaps:
  - name: "keysimulator"
    prefix: "keySimulator -k"
    codes:
### These codes need to be rcCodes.py enum list, being translated to what is required by the keySimulator app
      POWER: "power"
      HOME: "home"
      GUIDE: "guide"
      SKY: "sky"
      NUM_0: "0"
      NUM_1: "1"
      NUM_2: "2"
      NUM_3: "3"
      NUM_4: "4"
      NUM_5: "5"
      NUM_6: "6"
      NUM_7: "7"
      NUM_8: "8"
      NUM_9: "9"
      CHANNEL_UP: "chup"
      CHANNEL_DOWN: "chdown"
      UP: "up"
      DOWN: "down"
      LEFT: "left"
      RIGHT: "right"
      SELECT: "enter"
      MUTE: "mute"
      VOL_UP: "volup"
      VOL_DOWN: "voldown"
      PLAY: "play"
      PAUSE: "pause"
      FFORWARD: "fastfwd"
      REWIND: "rewind"
      RECORD: "record"
      RED: "red"
      GREEN: "green"
      YELLOW: "yellow"
      BLUE: "blue"
      PAGEUP: "pageup"
      PAGEDOWN: "pagedown"
      EXIT: "exit"
      SEARCH: "search"
      INFO: "info"
      APPS: "apps"
      ONDEMAND: "ondemand"
      HELP: "help"
      INPUTKEY: "inputkey"
      LOWBAT: "lowBat"

@Ulrond Ulrond changed the title Audit: Review rcCodes.py Audit: Review rcCodes.py & sendKey Nov 29, 2024
@TB-1993
Copy link
Contributor

TB-1993 commented Dec 6, 2024

More things to check in this review:

  • The prefix key in the key map yaml files.
    • This could be renamed to keyset.
    • A comment to explain it usage may be sufficient.
    • Alternatively, both prefix and keyset with their usage being interchangeable and keyset being the preferred key shown in the documentation.
  • Examples of different key maps should be committed to the example config directory.
  • The testing suites for the remoteController module should be upgraded to better test remoteController bring up.
    • It may be best to implement a test mode into the modules to allow this testing to work without need for additional hardware.
    • A test should be in place to validate keymaps and all keymaps submitted must be tested.
  • The remoteController's sendKey method should be upgraded to check the key argument given is an RCCode Enum and error out gracefully when not provided a valid key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants