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

stm32 and CO_init #25

Closed
mgiaco opened this issue Mar 21, 2017 · 15 comments
Closed

stm32 and CO_init #25

mgiaco opened this issue Mar 21, 2017 · 15 comments

Comments

@mgiaco
Copy link

mgiaco commented Mar 21, 2017

So, today I tried to compile the sample code for the STM32. There are a lot of build errors so I will try to fix them and add a pull as soon as possible. But first I had a question concerning CO_init(...)
So in the latest Version of the stack this function uses some fixed prototype of

CO_ReturnError_t CO_init(
        int32_t                 CANbaseAddress,
        uint8_t                 nodeId,
        uint16_t                bitRate)
{

So the problem here is that an STM32 has some different type here. It would be better to use CAN_TypeDef *CANbaseAddress, for STM32. But thats not possible because CO_init isn´t a driver part function. So what was your idea here? I the driver template there is some hit... could be different for other microcontroller. But then you use a fix int32_t.

The only solution here is to use a mapping in the driver...

CAN0 => CANbaseAdress 0x1 => mapps to CAN_TypeDef CAN1 in the function.
CAN0 => CANbaseAdress 0x2 => mapps to CAN_TypeDef CAN1 in the function.

or use some preprocessor magic.

cheers
mathias

@CANopenNode
Copy link
Owner

I try for CO_init to be the same for all microcontrollers. Otherwise there is necessary to add controller-specific code into CANopen.c file.

I think, mapping inside CO_init function is most elegant solution.

@mgiaco
Copy link
Author

mgiaco commented Mar 22, 2017

Hi, so what do you exactly mean with mapping inside CO_init?

I think there is some inconsistency. In the CANopen.c you use this prototype:

/**
 * Initialize CANopen stack.
 *
 * Function must be called in the communication reset section.
 *
 * @param CANbaseAddress Address of the CAN module, passed to CO_CANmodule_init().
 * @param nodeId Node ID of the CANopen device (1 ... 127).
 * @param nodeId CAN bit rate.
 *
 * @return #CO_ReturnError_t: CO_ERROR_NO, CO_ERROR_ILLEGAL_ARGUMENT,
 * CO_ERROR_OUT_OF_MEMORY, CO_ERROR_ILLEGAL_BAUDRATE
 */
CO_ReturnError_t CO_init(
        int32_t                 CANbaseAddress,
        uint8_t                 nodeId,
        uint16_t                bitRate);

Then you use in the PIC diver this prototype

CO_ReturnError_t CO_CANmodule_init(
        CO_CANmodule_t         *CANmodule,
        uint16_t                CANbaseAddress,
        CO_CANrx_t              rxArray[],
        uint16_t                rxSize,
        CO_CANtx_t              txArray[],
        uint16_t                txSize,
        uint16_t                CANbitRate);

So you pass a int32_t CANbaseAddress to an uint16_t CANbaseAddress function.
Do you get no warning here?

@CANopenNode
Copy link
Owner

Sorry, I meant mapping inside CO_CANmodule_init function. That is in the driver. For example as in 16bit PIC (line 178): https://github.com/CANopenNode/CANopenNode/blob/master/stack/PIC24_dsPIC33/CO_driver.c

You are right, there is some inconsistency, CANbaseAddress should be int32_t in all drivers. I will fix it.

@mgiaco
Copy link
Author

mgiaco commented Mar 23, 2017

Okay many thanks,
I will update the STM drivers i will add some pulls for the STM32F103 and STM32F407 in the near future.
cheers
mathias

@mgiaco mgiaco closed this as completed Mar 23, 2017
@mgiaco
Copy link
Author

mgiaco commented Mar 24, 2017

So to fix the inconsistency ... one solution could be to write a new header file as an example CO_driver_interface.h => here every function prototype and maybe also every define and typedef which is mandatory equal for every driver driver could be added to this header. And in der CO_driver.h the interface must be included. Otherwise you have to duplicate a lot of prototypes and other stuff.

@mgiaco
Copy link
Author

mgiaco commented Mar 24, 2017

Please think about to separate the stack and the drivers. If you have separate repositories for everything it would be much easier to use the project in a managed build environment.

CANopenNode
CANopenNode_driver_stm32 => f1, f2, f3, f4, f7
CANopenNode_driver_k20
CANopenNode_driver_pic => different pics
CANopenNode_driver_ecos
...
...

As an example if I would like to use STM32 I add the stack CANopenNode as submodule and the CANopenNode_driver_stm32 as a submodule. And then I do not have to exclude every driver and so on ...

It is only a point to think about...

cheers

@mgiaco mgiaco reopened this Mar 24, 2017
@CANopenNode
Copy link
Owner

I got a hint about separating the drivers last week also from someone else. And like the idea quite much.

I think, I will separate the drivers, as you suggested. I can't maintain drivers for all devices, so there is no sense to have all them in my repositories. I will only include links to known implementations. So the stack could stay clean and nice.

cheers

@mgiaco
Copy link
Author

mgiaco commented Mar 24, 2017

That sounds great. Do you know a good CANopen Host SW? I tried CANopen Device Monitor from Port.de but I am not able to use peak CAN in the eval mode on windows.

I would like to test my STM32 node.
many thanks

@CANopenNode
Copy link
Owner

Here is an effort to rewrite a Object dictionary part of CANopenNode. New version will have separate drivers. Interface won't change.

I'm currently using my CANopenSocket with client command interface and some tools from can-utils on Linux. There is also Python client, but I didn't test it yet.

@c0d3runn3r
Copy link

I think you linked the wrong thing. That's my issue for the pull request to add the K20 driver, I did not try to rewrite the OD.

@CANopenNode
Copy link
Owner

Above was some "off topic" discussion about splitting the drivers from the stack. New OD was also discussed. And driver for STM32. There will be a little reorganisation of the stack.

@mgiaco
Copy link
Author

mgiaco commented Apr 7, 2017

Concerning the reorganisation. Have you taught about a CO_drivers_interface.h which lives inside the stack. And every driver must support the functions from that interface. With that approach it is really clear which functions are needed inside the stack and which functions I have to support in the driver. With the current design there is a lot of duplicated drivers.h files around with a lot of duplicated code.

@CANopenNode
Copy link
Owner

inside /stack/drvTemplate/CO_driver.h are the necessary functions. PIC32 (and socketCAN) are most relevant examples.

@mgiaco
Copy link
Author

mgiaco commented Apr 8, 2017

Hi,
Yes I know that´s wasn´t that what I mean. I mean why noch add a CO_driver.h ... or I don´t know how to name it ... only once inside the stack. And every driver can use this.

@martinwag
Copy link
Collaborator

separation is handled in #108 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants