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

tib888 can impl #101

Closed
wants to merge 2 commits into from
Closed

tib888 can impl #101

wants to merge 2 commits into from

Conversation

burrbull
Copy link
Member

This old CAN implementation was done by @tib888 for stm32f103xx.

Maybe someone will be interested and test it.

@TheZoq2
Copy link
Member

TheZoq2 commented Aug 18, 2019

I don't have any experience with CAN buses, so I can't comment much on it unfortunately

@tib888
Copy link

tib888 commented Aug 18, 2019

It was a working and almost complete implementation. However it doesn't implement the embedded-hal CAN abstraction yet...

@TheZoq2
Copy link
Member

TheZoq2 commented Aug 24, 2019

However it doesn't implement the embedded-hal CAN abstraction yet...

IIRC, an embedded-hal CAN abstraction is considered out of scope for that project.

It was a working and almost complete implementation

Was as in "it's not working anymore" or "it hasn't been tested recently"? If it's the latter and one or two people with experience of CAN buses can comment on this, I wouldn't mind merging it :)

@arrowcircle
Copy link

Any ideas how to test it? It loosk like CAN is a real problem in embedded hal, it's not done after 3 attempts. So maybe it's better to have some implementation, then another and then conclude interface to embedded hal?

@TheZoq2
Copy link
Member

TheZoq2 commented Feb 4, 2020

Yea, sounds reasonable. The testing doesn't really have be thorough, just some indication that it does roughly what's expected :P

So if you use it for something, let us know how it works

@burrbull
Copy link
Member Author

burrbull commented Feb 4, 2020

unproven?

@arrowcircle
Copy link

@TheZoq2 Thing is with lack of docs and examples it's kinda hard to write an example.
How can I include this file in test project?

@TheZoq2
Copy link
Member

TheZoq2 commented Feb 5, 2020

Yea, lack of examples and docs certainly makes it harder, perhaps @tib888 could help answer some questions.

You probably first want to rebase this PR onto the current master, hopefully that should happen without conflicts.

git clone [email protected]:burrbull/stm32f1xx-hal.git
cd stm32f1xx-hal
git remote add upstream [email protected]:stm32/stm32f1xx-hal.git 
git rebase upstream/master

Then you can add that version of the crate to your project using

[dependencies.stm32f1xx_hal]
path = "path/to/stm32f1xx-hal

@burrbull
Copy link
Member Author

burrbull commented Feb 5, 2020

I've rebased, so now it should work with

[dependencies.stm32f1xx_hal]
git = "https://github.com/burrbull/stm32f1xx-hal"
branch = "can2"

@arrowcircle
Copy link

anyone got demo app working? Or something like test app?
What cases we need to check?

@tib888
Copy link

tib888 commented Feb 10, 2020

Well there is one example beside my original implementation:
https://github.com/tib888/stm32f1xx-hal/blob/master/examples/can-loopback.rs
That should be updated, and since it is a simple loopback test, could be run on a BluePill.

Unfortunately I do not have time to deal with this, but I plan revisit this in a few months.

@arrowcircle
Copy link

Little corrections

[dependencies.stm32f1xx-hal]
features = ["rt", "stm32f103", "medium"]
git = "https://github.com/burrbull/stm32f1xx-hal"
branch = "can2"

Also, we have a problem with compilation with error like:

>cargo build
   Compiling stm32f1xx-hal v0.5.3 (https://github.com/burrbull/stm32f1xx-hal?branch=can2#36dbdabc)
error[E0624]: method `mapr` is private
   --> .cargo\git\checkouts\stm32f1xx-hal-bc72ae77c0f80995\36dbdab\src\can.rs:351:14
    |
351 |         mapr.mapr()
    |              ^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0624`.
error: could not compile `stm32f1xx-hal`.

To learn more, run the command again with --verbose.

That is strange, because mapr method is public according to https://docs.rs/stm32f1xx-hal/0.2.1/stm32f1xx_hal/afio/struct.MAPR.html#method.mapr

@therealprof
Copy link
Member

You're using version 0.5.3 of the crate but looking at the 0.2.1 version of the documentation.

@burrbull
Copy link
Member Author

@arrowcircle try again

@arrowcircle
Copy link

arrowcircle commented Feb 11, 2020

@therealprof , @burrbull Thanks!

I was able to compile without errors and warnings. Here is example code:

#![deny(unsafe_code)]
#![no_std]
#![no_main]

use panic_halt as _;
use stm32f1xx_hal::{prelude::*, can};
use cortex_m_rt::{entry, exception, ExceptionFrame};
extern crate cortex_m_semihosting;
use cortex_m_semihosting::hio;
use core::fmt::Write;

#[entry]
fn main() -> ! {
    let config = can::Configuration {
        time_triggered_communication_mode: false,
        automatic_bus_off_management: true,
        automatic_wake_up_mode: true,
        no_automatic_retransmission: false,
        receive_fifo_locked_mode: false,
        transmit_fifo_priority: false,
        silent_mode: false,
        loopback_mode: true,
        synchronisation_jump_width: 1,
        bit_segment_1: 3,
        bit_segment_2: 2,
        time_quantum_length: 6,
    };

    let dp = stm32f1xx_hal::stm32::Peripherals::take().unwrap();
    let mut rcc = dp.RCC.constrain();

    let mut gpioa = dp.GPIOA.split(&mut rcc.apb2);
    let canrx = gpioa.pa11.into_floating_input(&mut gpioa.crh);
    let cantx = gpioa.pa12.into_alternate_push_pull(&mut gpioa.crh);

    //remapped version:
    //let mut gpiob = dp.GPIOB.split(&mut rcc.apb2);
    //let canrx = gpiob.pb8.into_floating_input(&mut gpiob.crh);
    //let cantx = gpiob.pb9.into_alternate_push_pull(&mut gpiob.crh);

    let mut afio = dp.AFIO.constrain(&mut rcc.apb2);
    //USB is needed here because it can not be used at the same time as CAN since they share memory:
    let mut can = can::Can::can1(
        dp.CAN1,
        (cantx, canrx),
        &mut afio.mapr,
        &mut rcc.apb1,
        dp.USB,
    );

    can.configure(&config);
    nb::block!(can.to_normal()).unwrap(); //just to be sure

    let id10: can::Id = can::Id::new_standard(10);
    let id11: can::Id = can::Id::new_standard(11);
    let id12: can::Id = can::Id::new_standard(12);
    let id13: can::Id = can::Id::new_standard(13);
    let id14: can::Id = can::Id::new_standard(14);
    let id15: can::Id = can::Id::new_standard(15);

    let filterbank0_config = can::FilterBankConfiguration {
        mode: can::FilterMode::List,
        info: can::FilterInfo::Whole(can::FilterData {
            id: id10.clone(),
            mask_or_id2: id11.clone(),
        }),
        fifo_assignment: 0,
        active: true,
    };
    let filterbank1_config = can::FilterBankConfiguration {
        mode: can::FilterMode::List,
        info: can::FilterInfo::Whole(can::FilterData {
            id: id12.clone(),
            mask_or_id2: id13.clone(),
        }),
        fifo_assignment: 1,
        active: true,
    };
    let filterbank2_config = can::FilterBankConfiguration {
        mode: can::FilterMode::List,
        info: can::FilterInfo::Whole(can::FilterData {
            id: id14.with_rtr(),
            mask_or_id2: id14.clone(),
        }),
        fifo_assignment: 0,
        active: true,
    };
    let filterbank3_config = can::FilterBankConfiguration {
        mode: can::FilterMode::List,
        info: can::FilterInfo::Whole(can::FilterData {
            id: id15.with_rtr(),
            mask_or_id2: id15.clone(),
        }),
        fifo_assignment: 1,
        active: true,
    };
    can.configure_filter_bank(0, &filterbank0_config);
    can.configure_filter_bank(1, &filterbank1_config);
    can.configure_filter_bank(2, &filterbank2_config);
    can.configure_filter_bank(3, &filterbank3_config);

    let mut hstdout = hio::hstdout().unwrap();

    let (tx, rx) = can.split();

    let (mut tx0, mut tx1, mut tx2) = tx.split();

    let txresult0 = tx0.request_transmit(&can::Frame::new(id10, can::Payload::new(b"0")));
    let txresult1 = tx1.request_transmit(&can::Frame::new(id11, can::Payload::new(b"1")));
    let txresult2 = tx2.request_transmit(&can::Frame::new(id12, can::Payload::new(b"2")));
    writeln!(
        hstdout,
        "tx: {:?} {:?} {:?}",
        &txresult0, &txresult1, &txresult2
    ).unwrap(); //while this printing slowly, all 3 messages are transfered
    let txresult0 = tx0.request_transmit(&can::Frame::new(id13, can::Payload::new(b"3")));
    let txresult1 = tx1.request_transmit(&can::Frame::new(id14, can::Payload::new(b"4")));
    let txresult2 = tx2.request_transmit(&can::Frame::new(id15, can::Payload::new(b"5")));
    writeln!(
        hstdout,
        "tx: {:?} {:?} {:?}",
        &txresult0, &txresult1, &txresult2
    ).unwrap(); //while this printing slowly, all 3 messages are transfered

    let (mut rx0, mut rx1) = rx.split();
    loop {
        if let Ok((filter_match_index, time, frame)) = rx0.read() {
            writeln!(
                hstdout,
                "rx0: {} {} {} {} {}",
                filter_match_index,
                frame.id().standard(),
                time,
                frame.data().len(),
                frame.data().data_as_u64()
            ).unwrap();
        };

        if let Ok((filter_match_index, time, frame)) = rx1.read() {
            writeln!(
                hstdout,
                "rx1: {} {} {} {} {}",
                filter_match_index,
                frame.id().standard(),
                time,
                frame.data().len(),
                frame.data().data_as_u64()
            ).unwrap();
        };
    }

    //writeln!(hstdout, "done.").unwrap();
}

#[exception]
fn HardFault(ef: &ExceptionFrame) -> ! {
    panic!("{:#?}", ef);
}

#[exception]
fn DefaultHandler(irqn: i16) {
    panic!("Unhandled exception (IRQn = {})", irqn);
}

and semihosting output

tx: Ok(()) Ok(()) Ok(())
tx: Ok(()) Ok(()) Ok(())
rx0: 0 11 0 1 49
rx1: 0 13 0 1 51
rx0: 2 14 0 1 52
rx1: 2 15 0 1 53

Does it look like a valid result?

@tib888
Copy link

tib888 commented Feb 12, 2020

@arrowcircle the output seems to be right:
the firs two line tells that successfully transmitted "0" "1" 2" and "3" "4" "5" payloads
the rest tells, that the ascii codes 49, 51, 52, 53 received with payload length == 1
trough the configured filter ids: 11, 13, 14, 15

  • at this point I'm not sure if every one of them are right. but you may simplify the example by commenting out many filters and messages and check if everything arrives well.
    the payload length if i remember well is about 8 bytes maximum.

@arrowcircle
Copy link

Should we convert this example to some kind of hardware running unit tests?
What cases should be checked first?

timokroeger added a commit to timokroeger/stm32f1xx-hal that referenced this pull request Apr 28, 2020
@timokroeger
Copy link
Contributor

I’m working on this to add support for the stm32f105/107 devices.
The CAN interface looks almost complete. Looking at the comment history following work items are outstanding:

  • Simple example
  • Documentation (cargo cannot build docs with this branch ATM)
  • Testing of filters (maybe this can be included in a second example)

I will open a new pull request once I completed them all.

@tib888
Copy link

tib888 commented Apr 28, 2020

I’m working on this to add support for the stm32f105/107 devices.
The CAN interface looks almost complete. Looking at the comment history following work items are outstanding:

  • Simple example
  • Documentation (cargo cannot build docs with this branch ATM)
  • Testing of filters (maybe this can be included in a second example)

I will open a new pull request once I completed them all.

That will be nice, thanks!
Please note that this implementation were made only for 103, but if I remember well 107 does support CAN2 besides CAN1. So lets check/add CAN2 implementation if you can!
Also It might be possible that on some devices the USB and CAN usage is not mutually exclusive so on those devices should be left out from the constructor parameters (device type dependent conditions needed).

@timokroeger
Copy link
Contributor

Indeed for stm32f105/107 CAN does not share memory with the USB peripheral.
But it has other tricky behavior: Filters for CAN2 are clocked by the CAN1 peripheral (ref. timokroeger@809327d)

@tib888
Copy link

tib888 commented Apr 28, 2020

Indeed for stm32f105/107 CAN does not share memory with the USB peripheral.
But it has other tricky behavior: Filters for CAN2 are clocked by the CAN1 peripheral (ref. timokroeger@809327d)

Nice! What about CAN related interrupts? Do they need some helper functions?

@timokroeger
Copy link
Contributor

Thank you for reminding.
I’ve not read up on the interrupt mechanism yet. Only saw there are some unimplented!() lines in the PR. That is another work item I’ll put on the list.

@arrowcircle
Copy link

Hey! Thanks for the contribution. I was experimenting with this code for 4 days and was struggling to get real communications working. Looks like I got malfunctioning bluepill, but I found this only on day 4 :(.

In general, this implementation does not have prescaler parameter to configure bit timing. And example does not show anything about speed clock and so on. I think we should have few examples:

  • Simple and quickstart code to easily check if everything compiles and works (tx, rx) in loopback mode without transceiver.
  • More complex example with clock configuration and custom CAN bus speed with real transceiver (or in loopback mode?).
  • Interrupts example
  • Full example (with 2 firmwares for tx and RX), more like a hardware test suite, that will require 2 boards with 2 transceivers. One board will initialize communication, second board will respond. In the end of the test suite if everything is fine - LED will light up if everything is ok on both boards.

Does it look sane?

@tib888
Copy link

tib888 commented Apr 29, 2020

Hey! Thanks for the contribution. I was experimenting with this code for 4 days and was struggling to get real communications working. Looks like I got malfunctioning bluepill, but I found this only on day 4 :(.

Yes, debugging HW is tricky even if you have a oscilloscope, like me :P.
In the recent days I also tried to establish CAN connection finally on two of mine prototype boards.
After on day I realized that the CAN driver chip on one of the boards was rotated 180 degree.
A day later I realized the TX side of the CAN driver chip on the other just do not work, so I had to replace it. (Not to mention, that when I ordered these SN65HVD230 CAN drivers, and tested them 2 years ago, it turned out that they are likely from a wrong batch so the VREF pin should be connected to the power, otherwise they do not work.)
But finally they do WORK!

In general, this implementation does not have prescaler parameter to configure bit timing. And example does not show anything about speed clock and so on.

This is a low level driver, more comfortable, but less capable abstractions like the one proposed in the embedded-hal crate can be built on the top of this later.
rust-embedded/embedded-hal#21
rust-embedded/embedded-hal#77
Until then, in the config struct lets look for these to configure bit timing:

/// Specifies the number of time quanta in Bit Segment 1: 1TQ..16TQ
/// defines the location of the sample point. It includes the PROP_SEG
/// and PHASE_SEG1 of the CAN standard. Its duration is programmable
/// between 1 and 16 time quanta but may be automatically lengthened
/// to compensate for positive phase drifts due to differences in the
/// frequency of the various nodes of the network.
pub bit_segment_1: u8,

/// Specifies the number of time quanta in Bit Segment 2: 1TQ..8TQ
/// defines the location of the transmit point. It represents the
/// PHASE_SEG2 of the CAN standard. Its duration is programmable
/// between 1 and 8 time quanta but may also be automatically
/// shortened to compensate for negative phase drifts.
pub bit_segment_2: u8,

/// Prescaling for time quantum: 1..1024
/// Length of a time quanta: tq = (BRP[9:0]+1) x tPCLK
pub time_quantum_length: u16,

@arrowcircle
Copy link

@tib888 Thanks for the answer. Could you please verify my thought?
From http://www.bittiming.can-wiki.info/

SJW = 1, Clock rate= 8Mhz, Sample Point = 87.5, speed = 500kbit/s

BitRate accuracy Pre-scaler Number oftime quanta Seg 1(Prop_Seg+Phase_Seg1) Seg 2 Sample Pointat RegisterCAN_BTR
500 0.0000 1 16 13 2 87.5 0x001c0000
500 0.0000 2 8 6 1 87.5 0x00050001

First row is preferred. Is it right, that I need to set bit_segment_1= 13, bit_segment_2= 2, time_quantum_length = 16 to have 500kbit/s?

What is the thing with SN65HVD230? Any links?

@timokroeger
Copy link
Contributor

Almost correct: time_quantum_length is a bit of misnomer as it actually defines the prescaler value.
In your case it should be time_quantum_length = 8MHz / (16 * 500kbit/s) = 1

@tib888
Copy link

tib888 commented Apr 30, 2020

14/(14+2) = 0.875
8_000_000/((14+2)*500_000) = 1

So I think you should fill the config struct like this:
config.bit_segment_1 =14
config.bit_segment_2 = 2
config.time_quantum_length =1

but the register will contain each value - 1:

    self.can.btr.modify(|_, w| unsafe {
        w.silm()
            .bit(config.silent_mode)
            .lbkm()
            .bit(config.loopback_mode)
            .sjw()
            .bits(config.synchronisation_jump_width)
            .ts1()
            .bits(config.bit_segment_1 - 1)
            .ts2()
            .bits(config.bit_segment_2 - 1)
            .brp()
            .bits(config.time_quantum_length - 1)
    });

@tib888
Copy link

tib888 commented Apr 30, 2020

What is the thing with SN65HVD230? Any links?

You need a CAN bus driver to connect your blue pill to the can bus.
For example: http://www.ti.com/lit/ds/symlink/sn65hvd230.pdf

@arrowcircle
Copy link

@tib888 I am about this part:

it turned out that they are likely from a wrong batch so the VREF pin should be connected to the power, otherwise they do not work

As I understand,

time_quantum_length should equal Prescaler (from the table)?

Also, why values of bit segments are substracted by 1? This looks really confusing, isn't it?

@tib888
Copy link

tib888 commented May 1, 2020

@tib888 I am about this part:>

it turned out that they are likely from a wrong batch so the VREF pin should be connected to the power, otherwise they do not work

If you order something cheap from China such things can happen...

As I understand,

time_quantum_length should equal Prescaler (from the table)?

cpu_clock / perscale gives you a clock, from this clock bit_segment_1 + bit_segment_2 pulses used during one bit detection on the can bus.
so the bit rate of can bus will be: cpu_clock / perscale / (bit_segment_1 + bit_segment_2)

Also, why values of bit segments are subtracted by 1? This looks really confusing, isn't it?

The allowed range of values is 1..16 or 1..8, and for the prescale 1..1024 (zero makes no sense).
If you want to store these on 4, 3, 10 bits you need to subtract one - this is the reason.

@arrowcircle
Copy link

@tib888

Now I am 100% confused.

14/(14+2) = 0.875
8_000_000/((14+2)*500_000) = 1

Where 14 comes from?

Sorry for stupid questions, but could you please explain how to properly init CAN BUS with values from http://www.bittiming.can-wiki.info/ ?

For example, lets get clock = 8MHz
Here is the table from the site.

Speed Prescaler NTQ TS1 TS2
1000kbit/s 1 8 6 1
500kbit/s 1 16 13 2
125kbit/s 4 16 13 2

for 1000 kbit/s

TQL = CLK / (NTQ * speed) = 8_000_000 / (8 * 1_000_000) = 1

for 500 kbit/s

TQL = CLK / (NTQ * speed) = 8_000_000 / (16 * 500_000) = 1

for 125 kbit/s

TQL = CLK / (NTQ * speed) = 8_000_000 / (16 * 125_000) = 4

Is it right?

If yes, how should CAN BUS initialize?

for 1000 kbit/s

let config = can::Configuration {
        time_triggered_communication_mode: false,
        automatic_bus_off_management: true,
        automatic_wake_up_mode: true,
        no_automatic_retransmission: false,
        receive_fifo_locked_mode: false,
        transmit_fifo_priority: false,
        silent_mode: false,
        loopback_mode: false,
        synchronisation_jump_width: 1,
        bit_segment_1: 7,
        bit_segment_2: 2,
        time_quantum_length: 2,
    };

for 500 kbit/s

let config = can::Configuration {
        time_triggered_communication_mode: false,
        automatic_bus_off_management: true,
        automatic_wake_up_mode: true,
        no_automatic_retransmission: false,
        receive_fifo_locked_mode: false,
        transmit_fifo_priority: false,
        silent_mode: false,
        loopback_mode: false,
        synchronisation_jump_width: 1,
        bit_segment_1: 14,
        bit_segment_2: 3,
        time_quantum_length: 2,
    };

for 125 kbit/s

let config = can::Configuration {
        time_triggered_communication_mode: false,
        automatic_bus_off_management: true,
        automatic_wake_up_mode: true,
        no_automatic_retransmission: false,
        receive_fifo_locked_mode: false,
        transmit_fifo_priority: false,
        silent_mode: false,
        loopback_mode: false,
        synchronisation_jump_width: 1,
        bit_segment_1: 14,
        bit_segment_2: 3,
        time_quantum_length: 5,
    };

@arrowcircle
Copy link

After few experiments, I found this configuration valid for 500 kbit/s

let config = can::Configuration {
        time_triggered_communication_mode: false,
        automatic_bus_off_management: true,
        automatic_wake_up_mode: true,
        no_automatic_retransmission: false,
        receive_fifo_locked_mode: false,
        transmit_fifo_priority: false,
        silent_mode: false,
        loopback_mode: false,
        synchronisation_jump_width: 1,
        bit_segment_1: 13,
        bit_segment_2: 2,
        time_quantum_length: 1,
    };

@timokroeger timokroeger mentioned this pull request May 9, 2020
4 tasks
@timokroeger
Copy link
Contributor

My attempts to continue this work can be found on my fork of the repo. I wasn’t able to get the receiver working.
I figured that I actually need more of a high level driver and started a draft implementation in #215.

@tib888
Copy link

tib888 commented May 10, 2020

My attempts to continue this work can be found on my fork of the repo. I wasn’t able to get the receiver working.
I figured that I actually need more of a high level driver and started a draft implementation in #215.

looking at the code, the documentation dded to be fixed here:
/// Returns true if the identifier is an extended identifier.
pub fn extended(&self) -> u32 {
(self.raw & 0b1111_1111_1111_1111_1111_1111_1111_1000) >> 3
}

And below that there is a comment, which I think valid: remove pub
// TODO: Does it need to be public?

@TheZoq2
Copy link
Member

TheZoq2 commented Aug 12, 2020

This seems to now be superseded by #215 so I'll go ahead and close, Feel free to re-open if that's the wrong call :)

@TheZoq2 TheZoq2 closed this Aug 12, 2020
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

Successfully merging this pull request may close these issues.

6 participants