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

Timestamped Frames #10

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4629e6f
Timestamped_Frame struct
equationcrunchor Apr 27, 2019
20f88aa
Generate timestamped structs
equationcrunchor Apr 27, 2019
abdaaa7
Handle timestamped frames
equationcrunchor Apr 27, 2019
235d4c3
Fix bugs
equationcrunchor Apr 27, 2019
cda72b5
Merge branch 'interrupts' into timestamped-frames
equationcrunchor Apr 27, 2019
e40b75c
CANlib_ReadFrame now checks if frame is valid
equationcrunchor Apr 27, 2019
cf24235
CANlib_HandleFrame now uses CANlib_ReadFrame
equationcrunchor Apr 27, 2019
0069c3e
Fix typos
equationcrunchor Apr 27, 2019
a11eb1c
Check both FIFOs
equationcrunchor May 1, 2019
fb6cf2e
Prevent unused warning
equationcrunchor May 1, 2019
c60f943
Merge remote-tracking branch 'origin/master' into timestamped-frames
equationcrunchor May 2, 2019
71c463e
Rename TimestampedFrame
equationcrunchor May 2, 2019
bff9a8b
Rename timestamp field to stamp
equationcrunchor May 2, 2019
3989e45
Change name
equationcrunchor May 4, 2019
d9404a3
Change ReadFrame function
equationcrunchor May 4, 2019
1377b3e
Change CANlib_update functions
equationcrunchor May 4, 2019
acc0883
Change CANlib_HandleFrame
equationcrunchor May 4, 2019
616a1f5
HandleFrame returns bool
equationcrunchor May 4, 2019
dd9d9db
New readframe functions
equationcrunchor May 4, 2019
cbd2495
Change HandleFrame
equationcrunchor May 4, 2019
0863f19
Fix mistakes
equationcrunchor May 4, 2019
ac7ab57
Change update can functions
equationcrunchor May 4, 2019
0cfa2d9
Fix typos in update_can and switch on instance
equationcrunchor May 4, 2019
827eeef
Extern C stuff
equationcrunchor May 4, 2019
04a93f5
Fix typo
equationcrunchor May 11, 2019
55267dc
Fix unused
equationcrunchor May 15, 2019
18be57b
Add comment
equationcrunchor Jul 25, 2019
f7dc936
Merge branch 'master' into timestamped-frames
equationcrunchor Jul 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion generator/bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def write(can, computers, output_path=bus_path):
raw_buses = set()
for computer in computers:
if not ('can' in computer.participation['name'].keys()):
# This computer neither sends nor recieves can messages
# This computer neither sends nor receives can messages
continue

raw_buses |= set(computer.participation['name']['can'].mapping.values())
Expand Down
61 changes: 43 additions & 18 deletions generator/computers_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
frame_handler, is_multplxd
from math import ceil

raw_bus_to_handle = {"CAN_1": "hcan1", "CAN_2": "hcan2", "CAN_3": "hcan3"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file must work with all drivers, even outside of STM.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These variables must be inferred from the spec.

raw_bus_to_instance = {"CAN_1": "CAN1", "CAN_2": "CAN2", "CAN_3": "CAN3"}

def single_handler(frame, name_prepends, num_tabs, fw):
tot_name = coord(name_prepends, frame.name, prefix=False)
fw(
'\t' * num_tabs + 'case CANlib_{}_key:'.format(tot_name) + '\n' +
'\t' * (num_tabs + 1) + 'CANlib_Handle_{}(frame);\n'.format(tot_name) +
'\t' * (num_tabs + 1) + 'CANlib_Handle_{}(ts_frame);\n'.format(tot_name) +
'\t' * (num_tabs + 1) + 'break;\n'
)

Expand Down Expand Up @@ -63,9 +65,16 @@ def write(can, computers, output_path=computer_c_dir_path):

with open(f_path, 'w') as f:
fw = f.write
fw('#include <time.h>\n')
fw('#include <stdbool.h>\n')
fw('#include <stm32f4xx_hal.h>\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not allowed.

fw('#include "pack_unpack.h"\n')
fw('#include "canlib_{}.h"\n\n'.format(computer.name))

fw('extern CAN_HandleTypeDef hcan1;\n')
fw('extern CAN_HandleTypeDef hcan2;\n')
fw('extern CAN_HandleTypeDef hcan3;\n\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not allowed.


fw(
'CAN_Raw_Bus_T CANlib_GetRawBus(CANlib_Bus_T bus) {\n'
'\tswitch (bus) {\n'
Expand All @@ -91,10 +100,11 @@ def write(can, computers, output_path=computer_c_dir_path):
fw('\t}\n}\n\n')

for busnm, bus in computer.participation['name']['can'].subscribe.items():
fw('static void CANlib_HandleFrame_{}(Frame* frame)'.format(busnm) + '{\n')

fw('static void CANlib_HandleFrame_{}(TimestampedFrame* ts_frame)'.format(busnm) + '{\n')
if any(is_multplxd(msg) for msg in bus):
fw('\tuint64_t bitstring;\n')
fw('\tswitch(frame->id) {\n')
fw('\tswitch(ts_frame->frame.id) {\n')

for msg in bus:
msg_handler(msg, busnm, fw)
Expand All @@ -108,27 +118,42 @@ def write(can, computers, output_path=computer_c_dir_path):

for busnm, bus in computer.participation['name']['can'].subscribe.items():
fw(
'static void CANlib_update_can_{}(void)'.format(busnm) + '{\n' +
'\tFrame frame;\n'
'static void CANlib_Update_can_{}(void)'.format(busnm) + ' {\n' +
'\tTimestampedFrame ts_frame;\n'
)

raw_bus = computer.participation['name']['can'].mapping[busnm]
instance = raw_bus_to_handle[raw_bus]

fw(
'\tCANlib_ReadFrame(&frame, {});\n'.format(busnm) +
'\tCANlib_HandleFrame_{}(&frame);\n'.format(busnm) +
'\tif (HAL_CANlib_ReadFrame(&{}, &(ts_frame.frame))) {{\n'.format(instance) +
'\t\tts_frame.stamp = HAL_GetTick();\n' +
'\t\tCANlib_HandleFrame_{}(&ts_frame);\n'.format(busnm) +
'\t}\n' +
'}\n\n'
)

fw('void CANlib_update_can() {\n')
for busnm in computer.participation['name']['can'].subscribe.keys():
fw('\tCANlib_update_can_{}();\n'.format(busnm))
fw('\tCANlib_Update_can_{}();\n'.format(busnm))
fw('}\n\n')

fw('void CANlib_HandleFrame(CAN_Raw_Bus_T raw_bus, Frame* frame) {\n')
fw('\tswitch(raw_bus) {\n')
for bus in computer.participation['name']['can'].subscribe.keys():
fw(
'\t\tcase {}:\n'.format(computer.participation['name']['can'].mapping[bus]) +
'\t\t\tCANlib_HandleFrame_{}(frame);\n'.format(bus) +
'\t\t\tbreak;\n'
)
fw('\t\tdefault:\n\t\t\tUNUSED(frame);\n\t\tbreak;\n')
fw('\t}\n}\n')
fw('void CANlib_HandleFrame(TimestampedFrame *ts_frame, time_t stamp, CAN_TypeDef* instance) {\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this function take a timestamped frame AND a timestamp? Whoever calls it should just assign the stamp. Also use T const * if you don't modify things.

if len(computer.participation['name']['can'].subscribe.keys()) > 0: # check if computer receives messages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not this part's job to make a function unusable if nobody subscribes to it. It should be generated and just not be exposed to the user.

fw('\tts_frame->stamp = stamp;\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove.

fw('\tswitch ((intptr_t)instance) {\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. The function should take a CANlib CAN_x instance. That way the driver can provide it and you don't need the raw_bus_to_instance.

for bus in computer.participation['name']['can'].subscribe.keys():
fw(
'\t\tcase (intptr_t){}:\n'.format(raw_bus_to_instance[computer.participation['name']['can'].mapping[
bus]]) +
'\t\t\tCANlib_HandleFrame_{}(ts_frame);\n'.format(bus) +
'\t\t\tbreak;\n'
)
fw('\t\tdefault:\n')
fw('\t\t\tbreak;\n')
fw('\t}\n')
else: # prevent unused warning
fw('\tUNUSED(ts_frame);\n')
fw('\tUNUSED(stamp);\n')
fw('\tUNUSED(instance);\n')
fw('}\n')
17 changes: 9 additions & 8 deletions generator/computers_h.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def declare_pub_frame(frame, name_prepends, fw):
def declare_sub_frame(frame, name_prepends, fw):
tot_name = coord(name_prepends, frame.name, prefix=False)
fw(
'extern CANlib_{}_T CANlib_{}_Input;\n'.format(tot_name, tot_name) +
'void CANlib_Handle_{}(Frame *frame);\n'.format(tot_name, tot_name)
'extern CANlib_{}_Timestamped_T CANlib_{}_Input;\n'.format(tot_name, tot_name) +
'void CANlib_Handle_{}(TimestampedFrame *frame);\n'.format(tot_name, tot_name)
)


Expand All @@ -26,7 +26,7 @@ def write(can, computers, output_path=computer_h_dir_path):
f_path = os.path.join(output_path, 'canlib_{}.h'.format(computer.name))

if not ('can' in computer.participation['name'].keys()):
# This computer neither sends nor recieves can messagess
# This computer neither sends nor receives can messages
continue

with open(f_path, 'w') as f:
Expand Down Expand Up @@ -54,7 +54,7 @@ def write(can, computers, output_path=computer_h_dir_path):
for frame in bus:
frame_handler(frame, bus_name, declare_pub_frame, fw)
except KeyError:
pass # No CAN messages sent by this board
pass # No CAN messages sent by this board

fw('\n')

Expand All @@ -64,12 +64,13 @@ def write(can, computers, output_path=computer_h_dir_path):
for frame in bus:
frame_handler(frame, bus_name, declare_sub_frame, fw)
fw('\n')
fw('void CANlib_update_can(void);\n')
fw('void CANlib_HandleFrame(CAN_Raw_Bus_T raw_bus, Frame* frame);\n')
fw('void CANlib_update_can(void); // for those who still lack CAN interrupts\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is u lowercase?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things common to all generated headers should be in a separate static header and not in the autogenerator. The generator can #include them.

fw('void CANlib_HandleFrame(TimestampedFrame *ts_frame, time_t stamp, CAN_TypeDef* instance);\n')
fw('bool HAL_CANlib_ReadFrame(CAN_HandleTypeDef *hcan, Frame* out);\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be agnostic to drivers.

fw('bool HAL_CANlib_ReadFrameFromFIFO(CAN_HandleTypeDef *hcan, uint32_t RxFifo, Frame* out);\n')
except KeyError:
pass # No CAN messages received by this board
pass # No CAN messages received by this board

fw('\n#ifdef __cplusplus\n} // extern "C"\n#endif // __cplusplus\n\n')

fw(endif(header_name))

8 changes: 5 additions & 3 deletions generator/send_receive.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ def define_pub_frame(frame, name_prepends, busnm, fw):
def define_sub_frame(frame, name_prepends, fw):
tot_name = coord(name_prepends, frame.name,
prefix=False)
fw('void CANlib_Handle_{}(Frame *frame)'.format(
tot_name, tot_name) + ' {\n' + '\tCANlib_Unpack_{}(frame, &CANlib_{}_Input);\n'.format(tot_name, tot_name) + '}\n\n')
fw('void CANlib_Handle_{}(TimestampedFrame *ts_frame) {{\n'.format(tot_name, tot_name))
fw('\tCANlib_{}_Input.stamp = ts_frame->stamp;\n'.format(tot_name))
fw('\tCANlib_Unpack_{}(&(ts_frame->frame), &(CANlib_{}_Input.msg));\n'.format(tot_name,tot_name))
fw('}\n\n')


def define_struct(frame, name_prepends, fw):
tot_name = coord(name_prepends, frame.name)
fw('{}_T {}_Input;\n'.format(
fw('{}_Timestamped_T {}_Input;\n'.format(
tot_name, tot_name))


Expand Down
11 changes: 10 additions & 1 deletion generator/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ def msg_handler(frame, name_prepends, fw):
fw('} ' + '{}_T;\n\n'.format(coord(name_prepends, frame.name)))


def timestamped_msg_handler(frame, name_prepends, fw):
fw('typedef struct {\n')
fw('\ttime_t stamp;\n')
fw('\t{}_T msg;\n'.format(coord(name_prepends, frame.name)))
fw('} ' + '{}_Timestamped_T;\n\n'.format(coord(name_prepends, frame.name)))


def write(can, output_path=structs_path):
header_name = '_CAN_LIBRARY_STRUCTS'

Expand All @@ -25,10 +32,12 @@ def write(can, output_path=structs_path):

fw(ifndef(header_name))
fw('#include <stdint.h>\n')
fw('#include <stdbool.h>\n\n')
fw('#include <stdbool.h>\n')
fw('#include <time.h>\n')
fw('#include "enum_atom.h"\n\n')

for bus in can.bus:
for msg in bus.frame:
frame_handler(msg, bus.name, msg_handler, fw)
frame_handler(msg, bus.name, timestamped_msg_handler, fw)
fw(endif(header_name))
12 changes: 11 additions & 1 deletion src/driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,19 @@

#include "drivers/inc/stm32f4xx.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this really shouldn't be here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is that each driver header includes driver.h at the bottom, after defining the native types. Then concepts like CANlib_TransmitFrame have a common name and behavior across drivers while still using the native error type.

#include "bus.h"
#include <stdbool.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a driver specific error type, we shouldn't need bool.


#ifdef __cplusplus
extern "C" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking. This should probably be on all .h files.

#endif // __cplusplus

CANlib_Transmit_Error_T CANlib_TransmitFrame(Frame *frame, CANlib_Bus_T bus);
void CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus);
bool HAL_CANlib_ReadFrame(CAN_HandleTypeDef *hcan, Frame* out);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please conform to the format above, making a new type for receive error.

bool HAL_CANlib_ReadFrameFromFIFO(CAN_HandleTypeDef *hcan, uint32_t RxFifo, Frame* out);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong here. We don't need the low level concept of a FIFO, so this function can just be static inline in the STM driver's .c file.

CAN_Raw_Bus_T CANlib_GetRawBus(CANlib_Bus_T bus);

#ifdef __cplusplus
} // extern "C"
#endif // __cplusplus

#endif // __DRIVER_H
12 changes: 11 additions & 1 deletion src/drivers/inc/stm32f4xx.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,22 @@
#include "bus.h"

#include <stdint.h>
#include <stdbool.h>

typedef uint32_t Time_T; // in ms
typedef HAL_StatusTypeDef CANlib_Transmit_Error_T;
typedef HAL_StatusTypeDef CANlib_Init_Error_T;

#ifdef __cplusplus
extern "C" {
#endif // __cplusplus

CANlib_Transmit_Error_T CANlib_TransmitFrame(Frame *frame, CANlib_Bus_T bus);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be here and driver.h should be included at the bottom.

void CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus);
bool HAL_CANlib_ReadFrame(CAN_HandleTypeDef *hcan, Frame* out);
bool HAL_CANlib_ReadFrameFromFIFO(CAN_HandleTypeDef *hcan, uint32_t RxFifo, Frame* out);

#ifdef __cplusplus
} // extern "C"
#endif // __cplusplus

#endif // __STM32F4XX_CAN_H
49 changes: 22 additions & 27 deletions src/drivers/src/stm32f4xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "driver.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should include the stm specific driver/inc/xxx.h

#include <stdint.h>
#include <string.h>
#include <stdbool.h>

extern CAN_HandleTypeDef hcan1;
extern CAN_HandleTypeDef hcan2;
Expand Down Expand Up @@ -47,36 +48,30 @@ HAL_StatusTypeDef CANlib_TransmitFrame(Frame *frame, CANlib_Bus_T bus) {
return HAL_CAN_AddTxMessage(hcan, &pHeader, frame->data, &pTxMailbox);
}

void CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus) {
CAN_Raw_Bus_T raw_bus = CANlib_GetRawBus(bus);
CAN_HandleTypeDef *hcan;
switch(raw_bus) {
case CAN_1:
hcan = &hcan1;
break;
case CAN_2:
hcan = &hcan2;
break;
case CAN_3:
hcan = &hcan3;
break;
default:
return;
}
static void HAL_CANlib_ConvertFrame(CAN_RxHeaderTypeDef* pHeader, uint8_t data[8], Frame *out) {
out->id = pHeader->IDE == CAN_ID_STD ? pHeader->StdId : pHeader->ExtId;
out->dlc = pHeader->DLC;
memcpy(out->data, data, 8);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use sizeof(data) instead of 8 since you gave the size in the parameter.

out->extended = pHeader->IDE == CAN_ID_EXT;
}

bool HAL_CANlib_ReadFrameFromFIFO(CAN_HandleTypeDef *hcan, uint32_t RxFifo, Frame* out) {
uint8_t data[8] = {};
CAN_RxHeaderTypeDef pHeader;
for (int fifo = 0; fifo < 2; fifo++) { // There are 2 receive FIFOs
if (HAL_CAN_GetRxFifoFillLevel(hcan, fifo) > 0) {
HAL_CAN_GetRxMessage(hcan, fifo, &pHeader, data);
frame->id = pHeader.IDE == CAN_ID_STD ? pHeader.StdId : pHeader.ExtId;
frame->dlc = pHeader.DLC;

memcpy(frame->data, data, sizeof(data));
frame->extended = pHeader.IDE == CAN_ID_EXT;
return;
}
if (HAL_CAN_GetRxFifoFillLevel(hcan, RxFifo) > 0) {
if (HAL_CAN_GetRxMessage(hcan, RxFifo, &pHeader, data) == HAL_OK) {
HAL_CANlib_ConvertFrame(&pHeader, data, out);
return true;
}
}
return false;
}


bool HAL_CANlib_ReadFrame(CAN_HandleTypeDef *hcan, Frame* out) {
for (uint8_t fifo = 0; fifo < 2; fifo++) {
if (HAL_CANlib_ReadFrameFromFIFO(hcan, fifo, out)) {
return true;
}
}
return false;
}
6 changes: 6 additions & 0 deletions src/static.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <stdint.h>
#include <stdbool.h>
#include <time.h>

typedef struct {
uint32_t id;
Expand All @@ -11,6 +12,11 @@ typedef struct {
bool extended;
} Frame;

typedef struct {
nistath marked this conversation as resolved.
Show resolved Hide resolved
time_t stamp;
Frame frame;
} TimestampedFrame;

#define LIMIT(name) \
static Time_T last_sent = 0; \
if (HAL_GetTick() - last_sent < name ## _period) return; \
Expand Down