Skip to content

get_intensity method is broken #15

Closed

Description

@brandonaltieri1

This method does not seem to work - it yields the exception attached below. I believe it should instead be:

def get_intensity(self) -> float:
  """Get the current intensity value in volts.
  
  :raises: ProberException if an error occured.
  :returns: The intensity value.
  """
  
  self.comm.send("siph:get_intensity")
  resp = Response.check_resp(self.comm.read_line())
  # prober returns the light intensity shifted by one decimal place to the right
  return float(resp.message()) * 1e-1

I am planning on using this library extensively in the future - are there any plans to add unit tests to this repo? This seems like a bug that could be caught easily enough by a simple unit test.

Thanks!

Image

Activity

  1. beltoforion commented on Feb 21, 2025

    @beltoforion
    Contributor

    We are currently working on test automation for the remote command subsystem. Primary focus is testing the remote commands but this will involve using the Python API. So it is likely that unit test will be added for the Python API in the future.

    As a workaround for this specific issue I would suggest to directly send the remote command via the python api. Specifically with the command:

    resp = prober.send_cmd("siph:get_intensity")

    You will have to parse the value from the response manually but it should allow you to get the required information.

    The defective function will be fixed as soon as possible. Your fix seems to be ok but i need to talk with the team about this specific function. It is missing some parameters that the underlying remote command has and you seem to have added an additional division by 10 probably because SENTIO is returning raw values from the controller. This is something we may also address but this requires more coordination to be backwards compatible.

  2. brandonaltieri1 commented on Feb 21, 2025

    @brandonaltieri1
    Author

    We are currently working on test automation for the remote command subsystem. Primary focus is testing the remote commands but this will involve using the Python API. So it is likely that unit test will be added for the Python API in the future.

    Amazing, thank you!

    As a workaround for this specific issue I would suggest to directly send the remote command via the python api. Specifically with the command:

    resp = prober.send_cmd("siph:get_intensity")

    You will have to parse the value from the response manually but it should allow you to get the required information.

    The defective function will be fixed as soon as possible. Your fix seems to be ok but i need to talk with the team about this specific function. It is missing some parameters that the underlying remote command has and you seem to have added an additional division by 10 probably because SENTIO is returning raw values from the controller. This is something we may also address but this requires more coordination to be backwards compatible.

    Will do, indeed I have a workaround that seems to be working for now

  3. added a commit that references this issue on Feb 26, 2025
  4. beltoforion commented on Feb 26, 2025

    @beltoforion
    Contributor

    I have applied a fix along the lines what you already proposed. However i cannot include the division by 10 in the python logic as our policy is that the python return value and the remote command return value should be the same.

  5. beltoforion commented on Feb 27, 2025

    @beltoforion
    Contributor

    issue should be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions