Skip to content

Update Odin-data's Python Adapter Metadata Request & Parse Mechanism - #512

Open
Ohisemega wants to merge 37 commits into
masterfrom
update_adapter_to_metadata_response_Fix409
Open

Update Odin-data's Python Adapter Metadata Request & Parse Mechanism#512
Ohisemega wants to merge 37 commits into
masterfrom
update_adapter_to_metadata_response_Fix409

Conversation

@Ohisemega

@Ohisemega Ohisemega commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Update the adapter's controller to track the hash version of the metadata
Fixes #409
Fixes #527
Fixes #342

@Ohisemega Ohisemega added this to the in-progress milestone Jun 4, 2026
@Ohisemega
Ohisemega requested review from JamesOHeaDLS and ajgdls June 4, 2026 10:17
@Ohisemega Ohisemega self-assigned this Jun 4, 2026
@Ohisemega Ohisemega added enhancement Next stage development or improvement of an existing feature or utility. Awaiting review/approval Has been addressed in a PR and needs to be approved or commented for rework by a reviewer labels Jun 4, 2026
@codecov

codecov Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.81250% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.01%. Comparing base (10415aa) to head (75cde21).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...thon/src/odin_data/control/odin_data_controller.py 78.75% 17 Missing ⚠️
python/src/odin_data/control/ipc_tornado_client.py 89.58% 5 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #512      +/-   ##
==========================================
+ Coverage   47.84%   48.01%   +0.17%     
==========================================
  Files          95       95              
  Lines        7673     7724      +51     
  Branches      670      669       -1     
==========================================
+ Hits         3671     3709      +38     
- Misses       3810     3823      +13     
  Partials      192      192

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Ohisemega Ohisemega changed the title Implement DerivedParameter Class Update Adapter's Metadata Response Jun 4, 2026
@Ohisemega
Implement DerivedParameter Class
b374dd2 
The class is designed to reimplement key functions in the BaseParameter class and insert metadata
@Ohisemega
Ohisemega requested a review from timcnicholls June 4, 2026 13:24
@Ohisemega Ohisemega closed this Jun 12, 2026
@Ohisemega Ohisemega reopened this Jun 12, 2026
@Ohisemega
Ohisemega marked this pull request as draft June 12, 2026 09:51
@Ohisemega
Ohisemega marked this pull request as ready for review June 16, 2026 15:05
@Ohisemega
Ohisemega requested a review from LuisFSegalla June 16, 2026 15:19
Ohisemega added 2 commits June 16, 2026 17:04
@Ohisemega
Refactor Metadata Implementation in Odin Adapter
b534e70 
Remove derived_parameter_tree.
Implement a splice_params_metadata() function to overlay a parameter JSON with metadata at the leaf-nodes.
Add logic to cache the metadata and the metadata hash from odin-data responses.
Add logic to track the latest metadata hash.
Add logic to request for status/config metadata when the hash value changes.
@Ohisemega
Merge branch 'master' into update_adapter_to_metadata_response_Fix409
4fefff7
@Ohisemega Ohisemega changed the title Update Adapter's Metadata Response Update Odin-data's Python Adapter Metadata Request & Parse Mechanism Jun 17, 2026
JamesOHeaDLS
JamesOHeaDLS requested changes Jun 17, 2026
View reviewed changes

@JamesOHeaDLS JamesOHeaDLS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Ohisemega

I've added my comments. I think we also really needs an associated set of tests to check that these tree interactions are doing what we expect

Comment thread python/src/odin_data/control/ipc_tornado_client.py Outdated
Comment thread python/src/odin_data/control/ipc_tornado_client.py Outdated
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
@Ohisemega
Ohisemega force-pushed the update_adapter_to_metadata_response_Fix409 branch from ad62e7c to b61ee55 Compare June 17, 2026 15:18
Ohisemega added 4 commits June 17, 2026 17:12
@Ohisemega
Minor Refactor
4933f81
@Ohisemega
More Refactoring
b61ee55
@Ohisemega
Fixes
b7da2a9 
Fix class name bug in IpcTornadoClient class.
Remove default arguments from recursive_splice() function.
Remove duplicate loop processing form recursive_splice() function.
Simplify the 'IF' statement within the update_loop() function of the OdinDataCOntroller class.
Add test_control file and test case for recursive_splice() function.

Fixes #409
@Ohisemega
Merge branch 'master' into update_adapter_to_metadata_response_Fix409
2bddacd
@Ohisemega
Ohisemega requested a review from JamesOHeaDLS June 17, 2026 19:36
@Ohisemega

This comment was marked as outdated.

Sign in to view
LuisFSegalla

This comment was marked as outdated.

Sign in to view

@Ohisemega
Minor Fixes
b285c7e 
Add documenting comments.
@Ohisemega
Remove Condition which Deletes Metadata
8b7b9f8 
Remove condition which deletes metadata from IpcTornadoClient's dictionary.
Ohisemega added 6 commits July 22, 2026 13:55
@Ohisemega
Update the IpcTornadoClient Unit Test Suite
c42e347
@Ohisemega
Append Prameters to Status Response
d73df96 
Add 'timing', 'connected' and plugins list to Response
@Ohisemega
Fix IpcTornadoClient Unit Test
259ea96 
Update IpcTornadoClient to reflect sub-parameter of status_request under status.
@Ohisemega
Minor Fix
544ecae 
Remove redundant resetting of IpcTornadoClient parameters structure.
@Ohisemega
Merge branch 'master' into update_adapter_to_metadata_response_Fix409
511e72f
@Ohisemega
Update to Align with Time-stamp Implementation
75cadd1 
Odin-data returns time-stamps rather than hash strings.
Updated odin-control to align with that change.
ajgdls
ajgdls requested changes Jul 24, 2026
View reviewed changes
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
@Ohisemega
Ohisemega requested a review from ajgdls July 25, 2026 10:14
Ohisemega added 2 commits July 25, 2026 11:04
@Ohisemega
Add Boolean Flag for Command Strucuture
500a5d8 
A change in the config/status metadata implies a change in the command strucuture.
Add a boolean flag which is set to true when the config/status is updated.
@Ohisemega
Fix Update Command Strcture Logic
a24b3c7 
If metadata has been received, then the command structure might have an update!
timcnicholls

This comment was marked as resolved.

Sign in to view

@Ohisemega
Add Commits
7cd6c10 
Fix erratic commits.
ajgdls

This comment was marked as resolved.

Sign in to view

@Ohisemega

This comment was marked as resolved.

Sign in to view
Ohisemega and others added 3 commits August 10, 2026 17:54
@Ohisemega
Minor Refactor
3444674 
Add more docstrings to python functions.
Remove redundant arguments in functions.
@Ohisemega
Fix Bug
2319a00 
Make current and previous timestamp variables lists which correspond to the number of IpcTornadoClients that are instantiated at startup!
@ajgdls
Documentation and formatting updates.
5f8a255
@Ohisemega

Ohisemega commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the docstrings @ajgdls. They describe the functions very well and as intended!

ajgdls
ajgdls requested changes Aug 13, 2026
View reviewed changes

@ajgdls ajgdls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Ohisemega this is now close. I noticed that command execution is not quite working as I expected though. I think the current mechanism does this:
put is called on a command execution.
execute callback puts the name of the command and the plugin on the callback queue.

I see no mechanism that results in the queued commands being sent down to the C++ applications. We can discuss this today at our meeting, it may be that now we do not need to queue commands any longer and can instead simply send them straight through to the C++ applications.

JamesOHeaDLS
JamesOHeaDLS reviewed Aug 13, 2026
View reviewed changes
Comment thread python/src/odin_data/control/odin_data_controller.py Outdated
@Ohisemega
Convert queue_command to send_command
d9fedc1 
Commands are no longer queued but execued as parsed.
@Ohisemega
Typo Fix
aef2dc0
@Ohisemega
Fix Bug
76cef29 
Fix Bug in odin_data_controller Unit test.
Fix typo bug in odin_data_controller.
@Ohisemega

Ohisemega commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Ohisemega this is now close. I noticed that command execution is not quite working as I expected though. I think the current mechanism does this: put is called on a command execution. execute callback puts the name of the command and the plugin on the callback queue.

I see no mechanism that results in the queued commands being sent down to the C++ applications. We can discuss this today at our meeting, it may be that now we do not need to queue commands any longer and can instead simply send them straight through to the C++ applications.

I've rectified this, thank you!

@JamesOHeaDLS

This comment was marked as resolved.

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

Reviewers

@LuisFSegalla LuisFSegalla LuisFSegalla approved these changes
@timcnicholls timcnicholls Awaiting requested review from timcnicholls
@JamesOHeaDLS JamesOHeaDLS Awaiting requested review from JamesOHeaDLS
@ajgdls ajgdls Awaiting requested review from ajgdls

Requested changes must be addressed to merge this pull request.

Assignees

@Ohisemega Ohisemega

Labels

Awaiting review/approval Has been addressed in a PR and needs to be approved or commented for rework by a reviewer enhancement Next stage development or improvement of an existing feature or utility.

Projects

None yet

Milestone

in-progress

5 participants

@Ohisemega @JamesOHeaDLS @timcnicholls @ajgdls @LuisFSegalla