Skip to content
This repository was archived by the owner on Jul 8, 2022. It is now read-only.

Backport #378: PipeBlob as command's in/out #477

Closed
wants to merge 3 commits into from
Closed

Backport #378: PipeBlob as command's in/out #477

wants to merge 3 commits into from

Conversation

@Ingvord
Copy link
Member

@Ingvord Ingvord commented Aug 21, 2018 •
edited
Loading

This PR backports #378 into tango-9-lts branch. Client may now use PipeBlob as command argument and server may return PipeBlob as command result.

@tango-controls tango-controls deleted a comment Aug 22, 2018
@tango-controls tango-controls deleted a comment Aug 22, 2018
@Ingvord Ingvord requested a review from bourtemb August 23, 2018 09:11
@Ingvord Ingvord force-pushed the backport-pipeblob-as-cmd-in_out branch from 881784c to f02d10e Compare August 28, 2018 08:00
@tango-controls tango-controls deleted a comment Aug 29, 2018
@Ingvord Ingvord force-pushed the backport-pipeblob-as-cmd-in_out branch from f02d10e to b3bad64 Compare August 31, 2018 09:48
@Ingvord Ingvord force-pushed the backport-pipeblob-as-cmd-in_out branch from b3bad64 to 4a62c8b Compare August 31, 2018 10:12
@tango-controls tango-controls deleted a comment Aug 31, 2018
Ingvord
Ingvord commented Aug 31, 2018
View reviewed changes
Copy link
Member Author

@Ingvord Ingvord left a comment

Choose a reason for hiding this comment

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

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 4

See all issues in SonarCloud

cppapi/client/devapi_base.cpp
// Now, try to execute the command
//

//TODO extract class hierarchy
Copy link
Member Author

@Ingvord Ingvord Aug 31, 2018

Choose a reason for hiding this comment

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

Code Smell Code Smell: Complete the task associated to this "TODO" comment. (cpp:S1135)

See it in SonarCloud

cppapi/client/devapi_base.cpp
}


//TODO all command_inout methods must be moved to DeviceProxy
Copy link
Member Author

@Ingvord Ingvord Aug 31, 2018

Choose a reason for hiding this comment

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

Code Smell Code Smell: Complete the task associated to this "TODO" comment. (cpp:S1135)

See it in SonarCloud

cppapi/client/devapi_base.cpp
Except::throw_exception(API_PipeNoDataElement, "No data in pipe!", "DeviceProxy::write_pipe()");
}

DevULong max, len;
Copy link
Member Author

@Ingvord Ingvord Aug 31, 2018

Choose a reason for hiding this comment

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

Code Smell Code Smell: Define each identifier in a dedicated statement. (cpp:SingleDeclarationPerStatement)

See it in SonarCloud

cppapi/client/devapi_base.cpp
// Prepare data sent to device
//

//TODO replace with createDevPipeData
Copy link
Member Author

@Ingvord Ingvord Aug 31, 2018

Choose a reason for hiding this comment

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

Code Smell Code Smell: Complete the task associated to this "TODO" comment. (cpp:S1135)

See it in SonarCloud

@bourtemb
Copy link
Member

bourtemb commented Feb 6, 2019

As it is implemented it is not convenient to use the DevicePipeBlob.
We would need to find a more user-friendly way to create and extract DevicePipeBlob.
Is this feature request still needed? It seems some users are using json strings as alternative...

@t-b t-b closed this Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bourtemb bourtemb Awaiting requested review from bourtemb bourtemb is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Ingvord @bourtemb @t-b