Arduino_STM32 icon indicating copy to clipboard operation
Arduino_STM32 copied to clipboard

Add HardwareCAN library by Phonog and his corrections to core&system of STM32F1

Open megadrifter opened this issue 8 years ago • 5 comments

megadrifter avatar Oct 13 '17 11:10 megadrifter

I would accumulate here some comments:

two changes that are still to do : 1---------------------- (documentation only) In file STM32F1/libraries/HardwareCAN/examples/HardwareCANexample/Changes.h, line 34 reads +{ return 1 ; } // Dummy ISR please change to +{ return 0 ; } // Dummy ISR

2----------------------- (useless file) file can.temp.c should be removed. It was a security copy of can.c.

3---------------------- add a parameter to the filter function to indicate whether is ext ID filter or standard. That parameter needs to be added in 4 places, can.h, can.c, HardwareCAN.h and HardwareCan.cpp. CAN_STATUS can_filter(CAN_Port* CANx, uint8 filter_idx, CAN_FIFO fifo, CAN_FILTER_SCALE scale, CAN_FILTER_MODE mode, uint32 fr1, uint32 fr2, int [b]extID[/b])

megadrifter avatar Oct 17 '17 15:10 megadrifter

This is super interesting, I was intending to use I2C for my pet project but this seems so much better.

A few notes:

  • The code directly under STM32F1/libraries seems misplaced (rcc.h, rcc_f1.c and usb.c)
  • I suspect that this can be, at least temporarily, be brute-forced into a library-only PR. Similar to #409
  • There are a few defines in HardwareCAN.h that seems to be unused leftovers PID_REQUEST, PID_REPLY....THROTTLE

lacklustrlabs avatar Dec 27 '17 00:12 lacklustrlabs

lacklustrlabs,

The code directly under STM32F1/libraries seems misplaced (rcc.h, rcc_f1.c and usb.c)
I suspect that this can be, at least temporarily, be brute-forced into a library-only PR. Similar to #409
sorry, I do not understand

There are a few defines in HardwareCAN.h that seems to be unused leftovers PID_REQUEST, PID_REPLY....THROTTLE

Yes, that's righ. This came out of Phonog's particular project. Yet I left it as it is, but it would be better to clean, as well as to put several methods from the example into the class. Unfortunately I have no free time for that now.

megadrifter avatar Dec 27 '17 07:12 megadrifter

When I was adding slave functionality to the Wire library I noticed that it is possible to override the default libmaple implementation with custom libmaple .cpp/.c files placed directly inside a standalone library. Normally this would result in 'multiple definition' linking errors, but the arduino build system seems to pick only one of the conflicting .o files and it picks library .o files over the core.

Edit: I might be wrong about the .o files, another explanation could be that the core file never gets compiled. The end result is the same though.

If I make a PR to your repository, do you have the time to merge it?

lacklustrlabs avatar Dec 27 '17 10:12 lacklustrlabs

Yes, sure. But with no checking :) You can also make improvements according to the first comment above.

By the way, the master repo has changed a lot, and I was suggested to fork from new repo and make all changes (add CAN) to it.

megadrifter avatar Dec 27 '17 13:12 megadrifter

I think this is not relevant anymore as the changes from @Phonog have been merged.

stevstrong avatar Nov 03 '23 10:11 stevstrong