Hugovdberg / PIconnect

A python connector to the OSISoft PI and PI-AF databases
MIT License
91 stars 41 forks source link

Send values #498

Closed gabrielaleon closed 3 years ago

gabrielaleon commented 4 years ago

Is there any function to send values back to PI? It would be really interesting. It would be able to extract data, work with it and send results to tags (predictions for example).

Hugovdberg commented 4 years ago

Currently there is no python wrapper for the functionality. There is the possibility in the underlying PI AF SDK though, so it should be possible. However, currently I don't have a system on which I can test writing, so I can't reliably implement it. I could have a look at the documentation, and try to give you some pointers how you could use PIconnect to call those functions. It might take a while though, because I have never looked at that part of the documentation and currently have a lot of other stuff going on as well.

Hypn0tical commented 4 years ago

Is there any progress regarding this topic? Sending values to PI would be really nice!

ldariva commented 4 years ago

Hi Hugo. I have interest in this feature and have a lot of PI system where we can test it. If you have interested we can work on it togheter.

ldariva commented 4 years ago

I am just taking a quick search on the code to understand how it works and I think that to send values we need to use the UpdateValue method in the AFSDK.dll. What do you think about trying to do that?

ldariva commented 4 years ago

I am trying to set up a development environment here but I am getting a decode error when ruinning the pipenv sync -d command. The first error message I get is "UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc6 in position 49: invalid continuation byte". I have already tried in different computers and always get the same message.

Hugovdberg commented 4 years ago

Sorry for the late reply, some help is definitely welcome, since I can't test it at all. I think the UpdateValue and UpdateValues methods are the right methods to call. I'm thinking we could add a class to PI.PIPoint that takes a pandas.Series with timestamps as the index and a write_mode argument that takes any of the AFUpdateOption values.

The difficulty is that you need to pass AFValue objects, which in turn need AFTime objects for the timestamps, so you'll need to convert some values before being able to write them. But perhaps this snippet might help you on the way to figure out how to do this. Note that I explicitly waiver all responsibility for any lost data or other unexpected damages using these methods, I cannot test it, and am just going by reading the documentation.

import PIconnect as PI

with PI.PIServer('TEST_SERVER_PLEASE') as server:
    point = server.search('PLEASE_SELECT_A_SENSIBLE_POINT_THAT_HAS_NO_VALUABLE_DATA')[0]
    raw_pi_point = point.pi_point
    value = 0
    timestamp = PI.AF.Time.AFTime('2020-08-06 00:00')
    uom = point.units_of_measurement
    value_status = 64 # 64==Questionable (see https://techsupport.osisoft.com/Documentation/PI-AF-SDK/html/T_OSIsoft_AF_Asset_AFValueStatus.htm)

    value = PI.AF.Asset.AFValue(value, timestamp, uom, value_status)
    write_mode = 2 # NoReplace (see https://techsupport.osisoft.com/Documentation/PI-AF-SDK/html/T_OSIsoft_AF_Data_AFUpdateOption.htm)
    response = raw_pi_point.UpdateValue(value, write_mode)
    if not response:
        print("It might have actually worked :-)")

The write_modes and value_statusses should probably be included in the PIConsts as separate enumerations.

ldariva commented 4 years ago

Hi Hugo, I have just made a pull request. I didn't think in sending a batch of values but just sending one value in the current timestamp which is something, at least for me, makes sense. I was not thinking in loading past or future data but this is something we can think about.

I am pretty new in using GitHub so forgive me if I didn't follow all the procedures correctly. I was not able to follow you instructions to create a development environment so I just created a virtual environment, installed PIconnect module using pip, and after that I manually replaced the PIconnet folder with the on I was working on.

Please take a look in my pull request and tell me what do you think about it.

ldariva commented 4 years ago

I implemented the UpdateValues (with S) to write multiple values but it is writing just the last value informed in the list. Taking a look in the AFSDK manual there is the observation:

"For successful data write through Buffer, this method requires that PI Buffer Subsystem (PIBufSS) needs to be correctly pre-configured with Buffering Manager. Currently, buffering data through PIBufSS has a limitation where error feedback from PI Data Archive cannot be returned to the caller.

Data write through Buffer will be fanned to Collective members."

I will take a look on the servers configuration. One option would be write one value at a time. But this doesn't seem to be a very elegant solution.

cunhavictor commented 3 years ago

Hi, Any updates on this ticket? I would be awesome to have this fully operational!

AlcibiadesCleinias commented 3 years ago

Hi @ldariva , it seems that git diff between your branch and the develop is a bit messy. A lot of changes that are not compatible with the titled feature pr you suggested.

So, the mess makes it impossible @Hugovdberg to approve merge, I guess.

Your diff: https://github.com/Hugovdberg/PIconnect/pull/540/files

I guess some of the changes should be reverted (e.g. docstring flushes, class AuthenticationMode removal, ...)

btw, thank you for your contribution, and looking forward to reverts and merge

ldariva commented 3 years ago

Hi Ivan,

It is really nice to receive some feedback about this work I have done. I don't have much experience in working with collaboration with GitHub and with CI/CD (e.g Travis). Maybe you could help me with that. When I submitted the pull request of my contributions last year I received an error message from Travis which I don't understand its meaning. I don't know if you could see it. It says " The build https://travis-ci.com/github/Hugovdberg/PIconnect/builds/178988420 failed. This is a change from the previous build, which passed." but I don't know where the problem is. I didn't do any changes (as far as I remember) in the original code. I just added some new functions and enumerations to wrap the update_value method from the PIAF SDK. I know that it works because I tested it on my company PI servers. I would appreciate some help on this. I have a question for you. You said that you have created a new branch with my contributions (I am ok with that) and made some contributions (solved issue #54 - add timeout option) and did a pull request. I saw that some checks are still waiting to be done (something related to the Travis CI/CD). I really need to study how this stuff works. Hope we can work together on this feature.

Regards PS: Sorry for the late reply. I am on vacation this week.

Em sex., 23 de jul. de 2021 às 14:06, Ivan @.***> escreveu:

Hi @ldariva https://github.com/ldariva , it seems that git diff between your branch and the develop is a bit messy. A lot of changes that are not compatible with the titled feature pr you suggested.

So, the mess makes it impossible to approve merge by @Hugovdberg https://github.com/Hugovdberg , I guess.

Your diff: https://github.com/Hugovdberg/PIconnect/pull/540/files

I guess some of the changes should be reverted (e.g. docstring flushes, class AuthenticationMode removal, ...)

btw, thank you for your contribution, and looking forward to reverts and merge

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Hugovdberg/PIconnect/issues/498#issuecomment-885777363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMW6QMLV2UFU7FKSAXVFJS3TZGOQXANCNFSM4JRNP7UQ .

AlcibiadesCleinias commented 3 years ago

Hi, @ldariva ;

error message from Travis which I don't understand its meaning.

As I can see and @Hugovdberg has written, there was a problem with the test pipline (I dnk what exact, coz I have not used to use Travis), so, i.e. with Travis workflow at all, and now @Hugovdberg merely migrate its test workflow from Travis to GitHub Actions because of the problem.

I didn't do any changes (as far as I remember) in the original code. I just added some new functions and enumerations to wrap the update_value method from the PIAF SDK

@Hugovdberg found an explanation for a lot of changes in the code that do not relate to the update_value feature: https://github.com/Hugovdberg/PIconnect/pull/540#issuecomment-886993112

I know that it works because I tested it on my company PI servers.

I guess, your solution works, but only 1 doubt I still have: https://github.com/Hugovdberg/PIconnect/pull/540#discussion_r675731558 In general, my comments were about the major code changes that occurred coz of causes explained above by @Hugovdberg

I saw that some checks are still waiting to be done (something related to the Travis CI/CD). I really need to study how this stuff works.

Yes, some checks have already been done and 1 still in pending state: codacy-coverage-reporter (seems the one depends on the future action and manual approval of @Hugovdberg in the Codacy) You can find the checks here: https://github.com/Hugovdberg/PIconnect/blob/develop/.github/workflows/ci.yml This file is automatically read by GitHub and GitHub Nodes proceed the manifest declared in a directory: /.github/workflow/<mainfest>. This pipeline and .yml relate to GitHub Actions as you may guess.

Regards and happy holidays (;

ldariva commented 3 years ago

Hi Ivan.

Thanks. I saw that my pr was closed and that we are moving on with your pr. Please keep me updated and let me know whether I can help with something else.

Regards, Leandro

Em ter., 27 de jul. de 2021 às 16:49, Ivan @.***> escreveu:

Hi, @ldariva https://github.com/ldariva ;

error message from Travis which I don't understand its meaning.

As I can see and @Hugovdberg https://github.com/Hugovdberg has written, there was a problem with the test pipline (I dnk what exact, coz I have not used to use Travis), so, i.e. with Travis workflow at all, and now @Hugovdberg https://github.com/Hugovdberg merely migrate its test workflow from Travis to GitHub Actions because of the problem.

I didn't do any changes (as far as I remember) in the original code. I just added some new functions and enumerations to wrap the update_value method from the PIAF SDK

@Hugovdberg https://github.com/Hugovdberg found an explanation for a lot of changes in the code that do not relate to the update_value feature:

540 (comment)

https://github.com/Hugovdberg/PIconnect/pull/540#issuecomment-886993112

I know that it works because I tested it on my company PI servers.

I guess, your solution works, but only 1 doubt I still have: #540 (comment) https://github.com/Hugovdberg/PIconnect/pull/540#discussion_r675731558 In general, my comments were about the major code changes that occurred coz of causes explained above by @Hugovdberg https://github.com/Hugovdberg

I saw that some checks are still waiting to be done (something related to the Travis CI/CD). I really need to study how this stuff works.

Yes, some checks have already been done and 1 still in pending state: codacy-coverage-reporter (seems the one depends on the future action and manual approval of @Hugovdberg https://github.com/Hugovdberg in the Codacy) You can find the checks here:

https://github.com/Hugovdberg/PIconnect/blob/develop/.github/workflows/ci.yml This file is automatically read by GitHub and GitHub Nodes proceed the manifest declared in a directory: /.github/workflow/. This pipeline and .yml relate to GitHub Actions as you may guess.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Hugovdberg/PIconnect/issues/498#issuecomment-887788509, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMW6QMPWTII3XPOKO6WSHZDTZ4EUVANCNFSM4JRNP7UQ .

Hugovdberg commented 3 years ago

@ldariva I hope you don't mind I decided to go with @AlcibiadesCleinias his approach, as it was much easier to reconcile with the existing code. If you feel your addition has more value on certain points than the other, please feel welcome to comment on how to improve that as well.

Also, where at first I was kind of alone in development I now have the luxury and burden of choosing between multiple implementations of the same feature. Please tell me where/how I can improve my communication skills, as I hope you're willing to contribute more to the project!

ldariva commented 3 years ago

Hi Hugo, Not at all. I am glad that this feature will be incorporated in the package. And as Ivan said, he based his pr on my pr, so my contribution will be incorporated anyway. Please count on me to keep working on this project.

Regards, Leandro

Em qua., 28 de jul. de 2021 às 03:15, Hugo Lapré @.***> escreveu:

@ldariva https://github.com/ldariva I hope you don't mind I decided to go with @AlcibiadesCleinias https://github.com/AlcibiadesCleinias his approach, as it was much easier to reconcile with the existing code. If you feel your addition has more value on certain points than the other, please feel welcome to comment on how to improve that as well.

Also, where at first I was kind of alone in development I now have the luxury and burden of choosing between multiple implementations of the same feature. Please tell me where/how I can improve my communication skills, as I hope you're willing to contribute more to the project!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Hugovdberg/PIconnect/issues/498#issuecomment-888041491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMW6QMMI4CKST5ZVCXYYXN3TZ6N7HANCNFSM4JRNP7UQ .

Hugovdberg commented 3 years ago

This should now be possible with #573 merged. If any issues arise with the newly implemented feature, feel free to open a new bug report specific to that problem.