Change souk_readout_tools recommended branch from dev/1.3.0 to dev/1.5.0#1080
Closed
sr-cdf wants to merge 2 commits into
Closed
Change souk_readout_tools recommended branch from dev/1.3.0 to dev/1.5.0#1080sr-cdf wants to merge 2 commits into
sr-cdf wants to merge 2 commits into
Conversation
Updated README to reflect the new branch version.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This PR updates the ukkid_controller documentation to recommend the newer souk_readout_tools branch (dev/1.5.0). However, it also includes substantial formatting/behavioral changes in ukkid_controller.py, which is a significantly broader scope than the PR title/description suggests.
Changes:
- Update
ukkid_controllerREADME to recommendsouk_readout_toolsbranchdev/1.5.0. - Reformat
ukkid_controller.py(imports/whitespace) and modify behavior in several control-flow areas (notably mocked sweep loops and aborters).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| socs/agents/ukkid_controller/ukkid_controller.py | Large reformat + behavior changes; includes issues in mock sweep loops and aborter return semantics. |
| socs/agents/ukkid_controller/README.md | Updates recommended souk_readout_tools branch and cleans up formatting/whitespace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+720
to
+724
| if session.status == 'running': | ||
| session.set_status('stopping') | ||
|
|
||
| if session.status != 'running': | ||
| return False, 'Aborted streaming RFSoc data.' |
Comment on lines
+806
to
+825
| while ((now - sweep_start_time) < loop_time_seconds): | ||
| now = time.time() | ||
| sweep_active_seconds = (now - sweep_start_time) | ||
| sweep_status_string = "MOCK: Data has been sweeping for %.1f seconds." % (now - sweep_start_time) | ||
|
|
||
| session.data = {"value": 'SESSION ' + sweep_status_string, | ||
| "timestamp": now} | ||
|
|
||
| # Format message for publishing to Feed | ||
| message = {'block_name': 'sweep_string', | ||
| 'timestamp': now, | ||
| 'data': {'value': 'FEED ' + sweep_status_string}} | ||
| self.agent.publish_to_feed('UKKID_feed', message) | ||
|
|
||
| # Also send this string to the log - discuss whether appropriate. | ||
| self.log.info(sweep_status_string) | ||
| time.sleep(1) | ||
| now = time.time() | ||
| self.log.info('MOCK: full_band_sweep: Sweep complete.') | ||
| return True, 'MOCK: full_band_sweep: Sweep complete.' |
Comment on lines
+1168
to
+1172
| if session.status == 'running': | ||
| session.set_status('stopping') | ||
|
|
||
| if session.status != 'running': | ||
| return False, 'Aborted full band sweep data.' |
Comment on lines
+1253
to
+1272
| while ((now - sweep_start_time) < loop_time_seconds): | ||
| now = time.time() | ||
| sweep_active_seconds = (now - sweep_start_time) | ||
| sweep_status_string = "Data has been sweeping for %.1f seconds." % (now - sweep_start_time) | ||
|
|
||
| session.data = {"value": 'SESSION ' + sweep_status_string, | ||
| "timestamp": now} | ||
|
|
||
| # Format message for publishing to Feed | ||
| message = {'block_name': 'sweep_string', | ||
| 'timestamp': now, | ||
| 'data': {'value': 'FEED ' + sweep_status_string}} | ||
| self.agent.publish_to_feed('UKKID_feed', message) | ||
|
|
||
| # Also send this string to the log - discuss wther appropriate. | ||
| self.log.info('LOG' + sweep_status_string) | ||
| time.sleep(1) | ||
| now = time.time() | ||
| self.log.info('narrow_band_sweep: Sweep complete.') | ||
| return True, 'narrow_band_sweep: Sweep complete.' |
Comment on lines
+1455
to
+1459
| if session.status == 'running': | ||
| session.set_status('stopping') | ||
|
|
||
| if session.status != 'running': | ||
| return False, 'Aborted narrow band sweep data.' |
Comment on lines
+56
to
57
| readout server software that runs on the RFSoCs). **Use branch dev/1.5.0** | ||
| for compatibility with the ukkid_controller agent. |
Author
|
Confused about reported changes that I didn't make so abandoning this for now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updated README to reflect the new branch version.
Description
changed readme.md
Motivation and Context
souk_readout_tools latest stable version is now 1.5.0
How Has This Been Tested?
its a readme
Types of changes
change the readme
Checklist:
I literally changed one number from 3 to 5 in two places