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

Added ability to query more temperatures in agent acq #730

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mjrand
Copy link
Contributor

@mjrand mjrand commented Aug 20, 2024

Added extra turbo Temperature queries in the agent acq. Added a new function in drivers.py that queries all available turbo temperatures.

Description

Added get_turbo_temperatures() in drivers.py that queries all available turbo temperatures.
Changed the acq in agent.py to use get_turbo_temperatures() instead of get_turbo_motor_temperature(),
Added powerstage, electronics, pumpbottom, and bearing temperatures to the data in acq.

Motivation and Context

There are multiple turbo temperatures and previously only the motor temperature was being queried.
Added functionality to allow us to see all of the available temperatures on grafana in case one of the previously unqueried temps were to cause an error.

How Has This Been Tested?

I tested this on daq-dev and was able to show that the agent ran acq just fine.
All of the above temperatures were able to be seen on grafana.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copy link
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

This is great, we were just talking about adding these temperatures to the readout today.

The tests are hanging up because the test for the acq process doesn't know about the queries/responses to these addresses. Can you add a representative query/response to them here:

responses = {'0010034602=?108': format_reply('000300'), # get_turbo_motor_temperature()
'0010030902=?107': format_reply('000800'), # get_turbo_actual_rotation_speed()
'0010030302=?101': format_reply('Err001'), # get_turbo_error_code()
}

(The tests will continue to hang in the CI until that's fixed. Working on a fix for that upstream in ocs so it'll be more obvious in future PRs.)

Comment on lines -101 to +105
"Error_Code": "Err001",
"Error_Code": "1",
Copy link
Member

Choose a reason for hiding this comment

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

Is this really what's coming back? It looks like a string is being stored in InfluxDB. I see "000000" for turbos running on satp1, so maybe let's format this as "000001"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. What happened I think is that I tested casting the error code to make it readable on grafana and it worked so I wrongly assumed it was a simple int. I tried to separate that error code testing from this PR so we could tackle it differently but I accidentally forgot to revert this change, my b!

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.

2 participants