-
Notifications
You must be signed in to change notification settings - Fork 38
1048 remove wait from signal w.set and replace with epics specific argument #1134
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
base: main
Are you sure you want to change the base?
1048 remove wait from signal w.set and replace with epics specific argument #1134
Conversation
…e_wait_from_SignalW.set_and_replace_with_EPICS_specific_argument Update local branch to include latest changes from main branch.
| r_protocol, r_pv = split_protocol_from_pv(read_pv) | ||
| w_protocol, w_pv = split_protocol_from_pv(write_pv) | ||
| protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols") | ||
| options = options |
Check failure
Code scanning / CodeQL
Redundant assignment Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the redundant assignment, simply remove the line options = options from within the _epics_signal_backend function in src/ophyd_async/epics/core/_signal.py (line 90). This line serves no functional purpose and its removal will not alter the program's behavior. No additional changes or imports are required.
| @@ -87,7 +87,6 @@ | ||
| r_protocol, r_pv = split_protocol_from_pv(read_pv) | ||
| w_protocol, w_pv = split_protocol_from_pv(write_pv) | ||
| protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols") | ||
| options = options | ||
| signal_backend_type = get_signal_backend_type(protocol) | ||
| return signal_backend_type( | ||
| datatype, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AI is right
| r_protocol, r_pv = split_protocol_from_pv(read_pv) | ||
| w_protocol, w_pv = split_protocol_from_pv(write_pv) | ||
| protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols") | ||
| options = options |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the detected problem, remove the line options = options at line 90. This assignment is redundant because options is already a function parameter and is not modified or used before being passed as-is to signal_backend_type. Removing it avoids confusion about potential usage, removes clutter, and complies with best practices regarding unused local variables. No additional imports, method definitions, or further changes are necessary for this fix. Only remove this assignment in the function _epics_signal_backend around lines 80-97 in src/ophyd_async/epics/core/_signal.py.
-
Copy modified line R95
| @@ -87,12 +87,12 @@ | ||
| r_protocol, r_pv = split_protocol_from_pv(read_pv) | ||
| w_protocol, w_pv = split_protocol_from_pv(write_pv) | ||
| protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols") | ||
| options = options | ||
| signal_backend_type = get_signal_backend_type(protocol) | ||
| return signal_backend_type( | ||
| datatype, | ||
| r_pv, | ||
| w_pv, | ||
| options, | ||
| ) | ||
|
|
||
|
|
coretl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some comments, all the Tango tests need fixing too...
| with attempt: | ||
| await _wait_for( | ||
| self._connector.backend.put(value, wait=wait), timeout, source | ||
| self._connector.backend.put(value, wait=True), timeout, source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self._connector.backend.put(value, wait=True), timeout, source | |
| self._connector.backend.put(value), timeout, source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then can you remove all trace of the wait argument from all SignalBackends
| super().__init__(datatype) | ||
|
|
||
|
|
||
| async def stop_busy_record( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to find all the places this function is used, and make sure the signal itself is constructed with wait=False
| # Put with completion will never complete as we are waiting for completion on | ||
| # the move above, so need to pass wait=False | ||
| await self.motor_stop.set(1, wait=False) | ||
| await self.motor_stop.set(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
motor_stop needs constructing with wait=False
| return signal_backend_type( | ||
| datatype, | ||
| r_pv, | ||
| w_pv, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| w_pv, | |
| w_pv, | |
| options |
| r_protocol, r_pv = split_protocol_from_pv(read_pv) | ||
| w_protocol, w_pv = split_protocol_from_pv(write_pv) | ||
| protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols") | ||
| options = options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AI is right
closes #1048