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

Commands bug fix #65

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Commands bug fix #65

wants to merge 3 commits into from

Conversation

AlvaroEzq
Copy link
Collaborator

@AlvaroEzq AlvaroEzq commented Sep 6, 2024

Fixing a couple of caen devices commands that didn't work.

  • channel.stat (commit 4bea0b4)
  • module.board_alarm_status (commit 0fb875d)
    The problem on both relies on the usage of the check_command_output_and_convert function, where the method argument is not the correct one.
    This PR fixes both commands with ad-hoc commits. It would be better to have a consistent usage of the check_command_output_and_convert function with the commands mapping, but for now this works...

@lobis
Copy link
Owner

lobis commented Oct 16, 2024

Hi @AlvaroEzq , let's try to merge this. Is this PR complete or are there more needed changes? I think we would need to test these changes against real hardware to see if they work. I think we should merge this as soon as this is done (maybe it's done already).

@AlvaroEzq
Copy link
Collaborator Author

Hi @AlvaroEzq , let's try to merge this. Is this PR complete or are there more needed changes? I think we would need to test these changes against real hardware to see if they work. I think we should merge this as soon as this is done (maybe it's done already).

I have been using this branch for a long time now with real hardware and it works fine. Maybe it would be better to have a consistent usage of the check_command_output_and_convert function with the commands mapping instead of these ad-hoc fixes but it works with no problems.

@AlvaroEzq AlvaroEzq marked this pull request as ready for review October 19, 2024 16:38
@AlvaroEzq AlvaroEzq requested a review from lobis as a code owner October 19, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants