FOE Implementation is complicated

elsp1991 commented 5 years ago

Going through the esc_foe files I found out some issues:

static/private function are missing the static prefix
in FOE_config requires to pass two references but actually one ref is included to the other
FOE_error is calling FOE_abort with code 0 that actually is not performing any action other than FOE_init.
There is no API function for the Application to throw FOE abort codes. For example FOE_ERR_NOTINBOOTSTRAP can only be thrown from the application level

nakarlsson commented 5 years ago

Good input, looking forward to your pull-request. Before proceeding, would you mind filling in the CLA. http://openethercatsociety.github.io/cla/cla_soem_soes.pdf

nakarlsson commented 5 years ago

The changes make sense, makes me think if we should take the opportunity. Are you implementing/using read also, this FoE implementation have been around from the beginning originally only designed to do FW upgrade and in the early beginning it was application/HW dependent.

I was wondering if, since we plan to change the APi abit, also should add a foe_read_cfg? Make foe_cfg have read and write members.

elsp1991 commented 5 years ago

Sorry I was on vacation. Would be nice If we align the API changes with the FOE_read functionality. Currently I am not using the read functionality but searching the EtherCAT documentation and some Beckhoff devices I found out that they are using it mainly to upload logs and error reports from the slaves (usually in text human readable format). So I was thinking to implement something similar in the near future.

Regarding the foe_read_cfg I think is not needed it can be fully covered from the current implementation. Instead I would propose to implement some kind of read/write access flags that belong to the file struct/class that will allow you to make something readable/writable and also define in which etherCAT state this is allowed so the FOE library can report the foe errors and AL_STATU_CODE erros like (NOT_IN_BOOTSTRAP)

elsp1991 commented 5 years ago

Also I am planning to be in frankfurt for the ETG event https://www.ethercat.org/memberarea/download/ETG_TechnicalCommittee_2019_09.pdf Would be nice to meet you there

nakarlsson commented 5 years ago

make sense and would be simple enough, can you give it a try and update the PR. I can test if we get the code in place.

I've to pass TC meeting, quite busy in projects.

nakarlsson commented 4 years ago

@elsp1991 , did you get a chance to look at concluding this patch with read support?

elsp1991 commented 4 years ago

@nakarlsson, I did had a look a while ago. Firstly I changed the read function cause was reading from the wrong memory address causing lockup, but then I was busy to continue. The only thing missing right now is a way to determine the end of file. Because with the current implementation you are defining the maximum amount of memory a file will take. So in case of read you will have to send all the available memory as you dont have a way to determine the end of file. So as the end of file is most of the times file specific I think I will have to implement a read function hook that is file specific like the write function. I will have time in the end of September to check this out.

nakarlsson commented 4 years ago

Hi @elsp1991, any update? have you had time to finalize it?

elsp1991 commented 4 years ago

Hi @nakarlsson unfortunately I am still in the same state as I described above.

OpenEtherCATsociety / SOES

FOE Implementation is complicated #63