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

Multiple improvements #135

Closed
wants to merge 21 commits into from
Closed

Conversation

mkatliar
Copy link

@mkatliar mkatliar commented Feb 21, 2021

Several improvements have been made, here are the main ones:

  • RAII-based subscription management
  • Return codes replaced by exception-based error handling in many places
  • Interfaces of RWSClient and RWSInterface made more usable
  • Removed unnecessary class nesting in some places when it makes sense
  • Merged functionality to add/remove modules to tasks
  • Added ZoneData class
  • Using override keyword when appropriate
  • Improved const-correctness

dorianleveque and others added 20 commits November 23, 2020 11:01
commit a61ee60
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Fri Jan 22 11:41:37 2021 +0100

    Member functions in RWSClient made const and static where appropriate.

commit 4ffd800
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Tue Jan 19 17:19:52 2021 +0100

    - Made RAPIDTaskExecutionState, MechanicalUnitType, and MechanicalUnitMode enum classes;
    - Added value_type in RAPIDAtomicTemplate;

commit cf00d68
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Sat Jan 16 15:04:59 2021 +0100

    Moved the Coordinate enum from the RWSClient class scope to the abb::rws namespace scope and made it an enum class. Passing Coordinate by value instead of by reference.

commit 79e5e85
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Wed Dec 30 01:36:24 2020 +0100

    * Added cast operator of atomic RAPID types to corresponding standard types.
    * Added override keyword in rws_rapid.h where appropriate.

commit 0aca963
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Sat Dec 26 15:29:13 2020 +0100

    Added ZoneData class

commit f0ff994
Author: Mikhail Katliar <cepstr@gmail.com>
Date:   Sat Dec 26 00:04:30 2020 +0100

    Pulling out nested classes.
…rws namespace scope and made it an enum class. Passing Coordinate by value instead of by reference.
…xception messages from RWSResult and POCOResult.
* Fix file path for loading modules

* Add comment
* RAII-based subscription management
* Exception-based error handling in many places
This was referenced Feb 21, 2021
@gavanderhoorn
Copy link
Member

Thanks for the PR 👍 much appreciated.

I've skimmed through the commits and there are indeed quite a few very nice improvements.

Both @jontje and I are unfortunately busy, so a proper review will take some time.

Lame excuse, I know :(, but I can't make it any better than it is.

Please know that tardy response does not mean lack of interest or appreciation.

@gavanderhoorn
Copy link
Member

I noticed you appear to have #127 in here (and possibly some of the others).

Not sure we should merge those as part of this PR.

* Extra information is added to exceptions thrown by RWSClient.
* RWSClient constructor sends a request to the server to check connection and initiate authentication.
* Implemented getIOSignals() in RWSInterface.
@mkatliar
Copy link
Author

mkatliar commented Feb 23, 2021

I noticed you appear to have #127 in here (and possibly some of the others).

Not sure we should merge those as part of this PR.

@gavanderhoorn we needed the module load feature implemented in 20380cb, therefore we had to merge it; and its previous commit is 7b732fd, which is the source commit for #127. Please tell me if you know the proper way of dealing with it.

Also I think I need to re-create this PR and specify a stable source branch instead of NoMagicAi:master (to which our team keeps committing).

@mkatliar
Copy link
Author

Superceded by #136 (using a stable source branch).

@mkatliar mkatliar closed this Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants