search

CANopenNode / CanOpenSTM32

CANopenNode on STM32 microcontrollers.
Other
275 stars 110 forks source link

Missing CO_new() in canopen_app_process ? #39

Closed midiwidi closed 5 months ago

midiwidi commented 1 year ago

I'm not running CanOpenSTM32 on an STM microcontroller, but I used it as a basis for a port. As a result I might miss something, but ...

It looks like that in case of a comms reset, memory of all CO objects is freed in canopen_app_process() (CO_app_STM32.c, line 219) but not allocated again before calling canopen_app_resetCommunication() in lin 221.

Should canopen_app_init() be called instead of canopen_app_resetCommunication() which does the allocation and calls canopen_app_resetCommunication() or is CO_delete() not meant to be in line 219?

Pascal8749 commented 1 year ago

I think you might be right. With the current code I get problems with a "Reset Communication" command. I just tried your suggestion and replaced "canopen_app_resetCommunication()" with "canopen_app_init()". As a result the "Reset Communication" command works now. I also think that this is the most logical way. Personally, I would expect a "Reset Communication" command to reset the communication. Sounds silly, but deleting the instance and recreating it with the given parameters would meet that expectation exactly.

HamedJafarzadeh commented 1 year ago

I will check that and update you here . Reset Comm should reset the communication. Maybe I missed something...

midiwidi commented 7 months ago

Hi Hamed. Have you had a chance to verify the issue?

HamedJafarzadeh commented 7 months ago

Hi @midiwidi,

Yes, I had a look today and you are right. The way that CO_RESET_COMM handled is wrong. I will mark this as a bug and fix it in the next release.

midiwidi commented 7 months ago

Thanks for checking Hamed.

a-leibold commented 6 months ago

Compared with the main_black.c in the CANopenNode example it seems there is no CO_Delet() necessary