ChargePoint / pydnp3

Python bindings for opendnp3 libary
Apache License 2.0
29 stars 33 forks source link

Configure the Outstation's database of input point definitions. #11

Open bpfrare opened 5 years ago

bpfrare commented 5 years ago

I have been trying change the point definitions.

But the points didn't change.

In the function:

@staticmethod
  def configure_database(db_config):

I changed the code

print('--------------------------------------')
print(db_config.binary[2].evariation)
db_config.binary[2].evariation = opendnp3.EventBinaryVariation.Group2Var2
print('--------------------------------------')
print(db_config.binary[2].evariation)

But the values didn't change:

--------------------------------------
EventBinaryVariation.Group2Var1
--------------------------------------
EventBinaryVariation.Group2Var1

Someone can help me?

Thanks!

anhnguyen-cpi commented 5 years ago

The C++ source code opendnp3/app/MeasurementInfo.h shows that the DefaultEventVariation and DefaultStaticVariation variables are static constant. It means we cannot modify cfg.svariation and cfg.evariation.

For more details about the db_config, you can look at the source code in deps/dnp3/cpp/libs/include at: asiodnp3/DatabaseConfig.h opendnp3/outstation/MeasurementConfig.h opendnp3/app/MeasurementInfo.h

bpfrare commented 5 years ago

Hi Thanks!

I am trying to figure out how configure the db_config (without changing the array).

I have been following the example in repository.

https://github.com/ChargePoint/pydnp3/blob/master/examples/outstation.py

I look at the source code in C++ and the configuration is changing the "db_config" array too.

https://github.com/automatak/dnp3/blob/2.0.x/cpp/examples/outstation/main.cpp#L50

Could you send an example of the configuration?

anhnguyen-cpi commented 5 years ago

My example is built based on the C++ example, so it is not surprised that we have the same configuration db_config function. However, I just tried to run the C++ code with some print statement like the way you did and exactly the same thing happened.

Again, based on the C++ source code, I don't think we can be able to modify evariation and svariation.

bpfrare commented 5 years ago

I ran the C++ code.

I changed the outstation file: https://github.com/automatak/dnp3/blob/2.0.x/cpp/examples/outstation/main.cpp#L44-L50

To:

`void ConfigureDatabase(DatabaseConfig& config) { // example of configuring analog index 0 for Class2 with floating point variations by default std::cout << "---------------------------------" << endl; std::cout << int(config.analog[0].evariation) << endl; std::cout << "---------------------------------" << endl; config.analog[0].clazz = PointClass::Class2; config.analog[0].svariation = StaticAnalogVariation::Group30Var2; config.analog[0].evariation = EventAnalogVariation::Group32Var4; std::cout << int(config.analog[0].evariation) << endl; std::cout << "---------------------------------" << endl;

}`

And the values changed!

`

0

3

`

anhnguyen-cpi commented 5 years ago

I just realized that I run the old version of the code in my local machine, I'll checkout the newest code and try to run it again.

bpfrare commented 5 years ago

I didn't find another way to configure the database.

I try to change these lines: https://github.com/ChargePoint/pydnp3/blob/master/src/asiodnp3/DatabaseConfig.h#L65-L69

to:

.def_property( "analog", [](const asiodnp3::DatabaseConfig &self) { return &self.analog;}, py::return_value_policy::reference )

But my compilation freeze in 99%......

pierrehebert commented 4 years ago

Hi,

I'm facing the same issue and as a consequence I'm unable to use pydnp3. Did you find a solution for this problem? My understanding is that this isn't linked with a constant declaration in the c++ source code, as the fields svariation and evariation are writable. Any hint ?

pierrehebert commented 4 years ago

I tried to add a setter method "setSVariation()" to avoid the direct field access, but without improvement. Could it be that the AnalogConfig instance (or it's container) is cloned somewhere inside pybind11 ? Printing the __hash__() value from python always reports the same value before and after the assignment, meaning the underlying object points to the same data, right?

One possible workaround is to change the default value for the svariation and evariation fields, although obviously this is not satisfying as all items will share the same configuration.

For the following particular case, it allows floats to be transmitted as floats (with timestamp), instead of being truncated to integers:

--- cpp/libs/include/opendnp3/app/MeasurementInfo.h (date 1513622949000)
+++ cpp/libs/include/opendnp3/app/MeasurementInfo.h (date 1575044879150)
@@ -106,8 +106,8 @@

    static const EventType EventTypeEnum = EventType::Analog;
    static const StaticTypeBitmask StaticTypeEnum = StaticTypeBitmask::AnalogInput;
-   static const event_variation_t DefaultEventVariation = EventAnalogVariation::Group32Var1;
-   static const static_variation_t DefaultStaticVariation = StaticAnalogVariation::Group30Var1;
+   static const event_variation_t DefaultEventVariation = EventAnalogVariation::Group32Var7;
+   static const static_variation_t DefaultStaticVariation = StaticAnalogVariation::Group30Var5;
 };

 struct CounterInfo : private openpal::StaticOnly
bpfrare commented 4 years ago

I didn't find a solution. I started to work in C++ code.

Nowadays, these project is linking a very old version of opendnp3.

Last release (21 Jun 2019)

Linking version (18 Dec 2017)

I don't know how much work it will take to upgrade.

pierrehebert commented 4 years ago

Indeed, it's pretty old, but the library itself doesn't seem to have evolved much since. It seems that the API is mostly the same. What I don't understand is how pydnp3 can be used if the database cannot be properly configured. Am I missing something?

bpfrare commented 4 years ago

It's a very good question...... If you are missing something, I am missing something too! I didn't find anyway to change the configuration in pydnp3.

anhnguyen-cpi commented 4 years ago

Sorry for getting back this late. I run the outstation and verified that cfg.svariation and cfg.evariation were not configured for pydnp3. Unfortunately, I couldn't find a solution for this issue. I don't think it's due to the old version; I'm working on updating the latest version of opendnp3 to pydnp3, and faced the same issue with cfg.svariation and cfg.evariation configuration.

pierrehebert commented 4 years ago

Running this snippet shows that writting to svariation is ok:

ai = db.analog[0]
ai.svariation = opendnp3.StaticAnalogVariation.Group30Var5
print(ai.svariation) # outputs "StaticAnalogVariation.Group30Var5"

But the following code doesn't work:

db_config.analog[0].svariation = opendnp3.StaticAnalogVariation.Group30Var5
print(db_config.analog[0].svariation) # outputs "StaticAnalogVariation.Group30Var1"

The AnalogConfig instance is probably cloned when accessing it from the openpal::Array. Hence the local copy of AnalogConfig is correctly modified but the original data in the analog array in the DatabaseConfig instance remains untouched. Unfortunately the array doesn't support item assignment, and there's no constructor on AnalogConfig so it's not possible to do a workaround like this:

ai = opendpn3.AnalogConfig()
ai.clazz = ...
ai.svariation = opendnp3.StaticAnalogVariation.Group30Var5
ai.evariation = ...
db_config.analog[0] = ai
shipmints commented 4 years ago

Greetings and thank you to those who have spent time on this as I have now after having wasted hours trying to figure out why database configuration isn't working in pydnp3.

I'm curious if pydnp3 is an actively maintained or even truly used? Who might be working on it is it @ainguyen-cpi ? How can high-profile projects such as https://github.com/VOLTTRON/volttron (via https://github.com/craig8 and https://github.com/rcalvert-cpi @rcalvert-cpi ) and https://github.com/GRIDAPPSD/gridappsd-dnp3 from (via https://github.com/singha42) be taken seriously if pydnp3 doesn't allow one to properly configure the basic opendnp3 database?

I tried to find a test in the pydnp3 code base that illustrates database configuration tests but it seems that code wasn't tested?

Has anyone been able to work around this in python, perhaps calling a working C++ function that configures the database correctly but leaving the rest of the code in python?

shipmints commented 4 years ago

Still digging to understand what's going on. Taking a look at https://github.com/VOLTTRON/volttron/commit/5c2063ae93ff999607a4d3ad69ec7dd88422c3c5#diff-44aae731e80cb08c1a1c361e8f4ef628 it seems @anhnguyen-cpi knew in February 2019 that this was the case per the change:

# cfg.svariation and cfg.evariation are static const: cannot modify

Very curious how volttron/mesa dnp3 tests pass if input from output groups are not differentiable in the pydnp3 database? It seems all the rest of the code dealing with point configuration is moot if basics aren't working? Confused and concerned.

MortalAndTry commented 4 years ago

This is a key issue, is there a solution?

shipmints commented 4 years ago

My suggestion is just go ahead and with the simplest outstation database you need and test your software against your target master system. It will likely work fine despite the broken interfaces. If you need more contemporary opendnp3 features like support for octet strings, you'll have to wait for an update to pydnp3 which seems to have gone fallow. There is also an issue with pybind11 and you should consider using a more modern pybind11 as we were forced to do. These manifest themselves as broken ForeachItem / Foreach(visitor) wrappers.

pierrehebert commented 4 years ago

It's not likely to work fine if using anything different from integers, this is pretty limiting. Floats won't work, and I wouldn't call this an advanced opendnp3 feature.

txjmb commented 4 years ago

I spent some time working on this and finally decided it was going to be a pain, so I switched gears and redid the whole thing in CPPYY instead of PyBind11. So far, everything is working well, including being able to change these point variations, and it requires MUCH less code, as cppyy automatically generates all the wrapper code. I will publish the project in the next week or so. It won't be packageable in the first posting, but you will be able to build it and use it. I've got to figure out all the ins and outs of packaging a cppyy project. Happy to have some help if anyone wants to pitch in. Any preferences for a project name? Pydnp3 is taken, of course, as is pyopendnp3, so I was just going to go with pyod3 unless someone can come up with a better name.

shipmints commented 4 years ago

@txjmb, sounds good especially if cppyy adds no performance or debugability penalties. You could even wrap the most recent opendnp3 (which you may already have) rather than the old branch used in pydnp3 with two years of known bugs, poor support for octet strings, etc. etc. Is the cppyy version code-compatible with the pybind11 version? Let me know if you want a tester, we'd love the more recent opendnp3 and were planning to do some work on it when 3.x was finalized (they're restructuring the code, finally; was quite, um, over engineered).

txjmb commented 4 years ago

So far, performance is great. The only performance penalty that I know of is on startup--it takes a couple of seconds to compile. Going to cppyy will make it much easier to go to 3.x. No changing tons of binding code. The devs of cppyy are very responsive. I found a bug and they gave me a workaround in a couple of hours and a complete fix in a day or so.

I've got it running against the latest opendnp3 master. There are some minor changes to the examples required to suppprt the changes in opendnp3. I'll include the new examples when I publish the repo, which will hopefully be later this week. The name will probably be dnpy.

Would appreciate you testing. If you are good at writing pytest code, that would be great, too. I will try to just lift the existing pytest code from pydnp3 for now. Obviously, we know one test we want to write--testing database configuration...

txjmb commented 4 years ago

Update... having issues getting the Master example to work properly. As soon as I get that working, I will publish the repo.

shipmints commented 4 years ago

Great. Thank you. I will be testing on larger unix/mac boxes. Not sure if the cppyy toolchain is Raspberry pi or small-computer friendly where the pydnp3 wrapper may be more appropriate. I can see a case potentially for both if cppyy proves too heavyweight.

txjmb commented 4 years ago

I agree. I forked the pydnp3 repo and updated it with the latest build of opendnp3 and pybind11. It works, but I wasn't able to fix the issue with the database updating (I'm sure it's possible, but it's a pain, which is why I went with cppyy. You're welcome to check that one out if you want to. Cppyy-based outstation is working fine. I'm having some strange behavior on Master demo that I can't yet explain. Haven't had enough time to dig into it too much, since what we needed most was the outstation.

shipmints commented 4 years ago

Nice work to get the underlying library upgraded. We switched to the newer pybind11 in our fork as traversing results e.g., ForEach was broken (I really do not understand how people use the current pydnp3 being as broken as it is). We've worked around the bugs that were stopping us and are stable with our fork, for the moment. We use both master and outstation in production as we have a fairly complex deployment. We also use master and outstations together as a test environment which you may also need, so it makes sense to get the master working. If we can help, let us know.

On the database configuration issue that started this whole thread, I'm not even convinced it matters since the support for analog and binary coding (nearly 100% of most uses), does automatic conversion under the covers so pedantic specificity may not even matter?

txjmb commented 4 years ago

@shipmints as you probably noticed, opendnp3 has officially released version 3.0. I was able to get master working with cppyy in an evening. I'll test outstation tomorrow and put it out in a github repo for people to work with.

Opendnp3 3.0 is so much simpler to work with. Thanks to cppyy, all I had to do was update the master header file and update the Python example file to match the slightly changed api and it worked right away. Cppyy dev fixed a bug that I had to change the opendnp3 cpp+ code slightly to work around, so now it works straight from the Opendnp3 master.

shipmints commented 4 years ago

Nice work, @txjmb looking forward to testing it. We'll schedule an official upgrade to opendnp3 3.x series for our use sometime later this year and may indeed switch to cppyy if it works well.

shipmints commented 4 years ago

Hi, @txjmb we're still interested in helping test the cppyy approach if you have time to share it.

shekharshank commented 4 years ago

Hi @txjmb, thanks for your work on pydnp3. I am facing similar issue, do you plan to share your repository?

shipmints commented 4 years ago

@txjmb we are now at a point where, in production, we have issues with pydnp3 on the old opendnp3. It would be fruitful to share the cppyy work if you're still willing. Would be good to have another set of test cases we have both masters and slaves and a fairly high throughput use case that's breaking.

txjmb commented 4 years ago

Sure. Maybe you can help me get it over the finish line. It works perfectly well until I try to turn it into a Python package. I am not familiar enough with CMake to understand exactly why the pathing errors are happening. I can upload a new version of the not-package-ready repo this weekend.

Michael

On Tue, Jun 30, 2020, 2:01 PM shipmints notifications@github.com wrote:

@txjmb https://github.com/txjmb we are now at a point where, in production, we have issues with pydnp3 on the old opendnp3. It would be fruitful to share the cppyy work if you're still willing. Would be good to have another set of test cases we have both masters and slaves and a fairly high throughput use case that's breaking.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChargePoint/pydnp3/issues/11#issuecomment-651984053, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPE6VRHWB2AOPXJLZQ4LZ3RZIZCDANCNFSM4G5BWMJA .

shipmints commented 4 years ago

Great. If we can help, we will.

txjmb commented 4 years ago

Finishing up making sure the Master.py example works properly. Have one issue I'm trying to fix. Outstation works fine, and Master works without custom SOEHandler. Will post as soon as I get it fixed.

shipmints commented 4 years ago

@txjmb due to a production issue, we wound up hacking opendnp3 to force point data types until either pydnp3 correctly supports changing databases or we can get cppyy working. Any time you're ready, we can help test.

txjmb commented 4 years ago

I've figured out most of the remaining issues. The last one has to do with callbacks. I think I know what's causing it and will finish posting as soon as I have that working.

txjmb commented 4 years ago

@shipmints I've uploaded the project here: https://github.com/txjmb/dnpy. Let me know if you have any questions. Still working out some of the custom callback/virtual method override code in the examples. But, most of it should work well for you.

txjmb commented 4 years ago

@shipmints the project now installs as a package. I haven't put it out on pypi yet, as there are a few things I'd like to figure out. But, it appears to be working except for the callbacks (mostly in master). Not sure what you are using it for, but any testing you could provide would be appreciated. I have put some initial install instructions out there. Let me know if you run into any issues (by posting an issue on the project).

shipmints commented 4 years ago

Hi @txjmb we will take a look. We use both master and outstation features and the callbacks are key to full functionality. I suspect the cppyy implementation's crossing the python/C++ barrier is the issue and some hints/wrappers may be in order. Let's continue discussion at https://github.com/txjmb/dnpy

EduardoRonchi commented 2 years ago

Any news on this matter? Any updates to pydnp3 making float valueas available or some workaround?

shipmints commented 2 years ago

@EduardoRonchi take a look at the work done in this recent fork https://github.com/edf-re/pydnp3 I haven't checked to see if they've fixed this bug (a quick glance says they haven't). In our application, we've worked around this issue though it remains a bit of a pain. Perhaps referencing this issue over there, they will be kind enough to address this as they probably need this fix themselves even if they don't know it yet.

LucaGalvagno commented 2 months ago

I solved the problem modifying src/asiodnp3/DatabaseConfig.h adding a method with a lamba function ("setBinaryConfig") . The pybind11 library has some limitations when it comes to binding templated classes or structs like EventConfig.

Below the changes :

.def_property_readonly( "binary", [](const asiodnp3::DatabaseConfig &self) { return &self.binary;}, py::return_value_policy::reference ) .def("setBinaryConfig", [](asiodnp3::DatabaseConfig &self, uint16_t index, opendnp3::EventBinaryVariation _evariation,opendnp3::PointClass _clazz ,opendnp3::StaticBinaryVariation _svariation) { self.binary[index].evariation=_evariation; self.binary[index].clazz=_clazz; self.binary[index].svariation=_svariation; }, py::arg("index"), py::arg("_evariation"), py::arg("_clazz") ,py::arg("_svariation"))