Timestamped Frames by equationcrunchor · Pull Request #10 · MITMotorsports/CANlib

equationcrunchor · 2019-04-27T19:55:32Z

Uses HAL_GetTick to put a timestamp on received CAN messages. Includes changes from the interruptsbranch. See sensor-node-timestamped-frames branch in MY19 for example usage in sensor node. If using interrupts, one should pass the correct raw bus to CANlib_HandleFrame in the callback. This will break existing code that uses CANlib.

nistath

There are several things missing:

No support for interrupt user to provide their own frame and timestamp.
No support for interrupt user to provide their own HAL frame and timestamp and have an automatic conversion.
Timestamp is not time of unpacking, it's time of reception.

nistath · 2019-05-02T04:26:24Z

generator/send_receive.py

-    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_{}(Timestamped_Frame *ts_frame) {{\n'.format(tot_name, tot_name))
+    fw('\tCANlib_{}_Input.timestamp = HAL_GetTick();\n'.format(tot_name))


This should be coming in as a function parameter. Devices could be buffering frames on their own and have the timestamp be in the past.

src/static.h

nistath · 2019-05-02T04:30:10Z

src/drivers/src/stm32f4xx.c

 }

-void CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus) {
+bool CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus) {


If it returns anything it should probably be HAL_StatusTypeDef.

The thing is HAL_CAN_GetRxMessage only returns HAL_OK or HAL_ERROR anyway; and also it could possibly not even be called if both FIFO's are empty

nistath · 2019-05-02T04:32:15Z

src/driver.h


 CANlib_Transmit_Error_T CANlib_TransmitFrame(Frame *frame, CANlib_Bus_T bus);
-void CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus);
+bool CANlib_ReadFrame(Frame *frame, CANlib_Bus_T bus);


Maybe introduce CANlib_Receive_Error_T, idk if we need to mess with this now tho

nistath · 2019-05-02T04:35:18Z

generator/computers_c.py


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


should be _Update and also missing space before {.

nistath · 2019-05-02T04:35:47Z

generator/computers_c.py

+                    'static void CANlib_update_can_{}(void)'.format(busnm) + '{\n' +
+                    '\tTimestamped_Frame ts_frame;\n'
+                )
+                if any(is_multplxd(msg) for msg in bus):


Why is this here? Unused

nistath · 2019-05-02T04:36:34Z

generator/computers_c.py

+                    fw('\tuint64_t bitstring;\n')
+
+                fw(
+                    '\tif (CANlib_ReadFrame(&(ts_frame.frame), {})) {{\n'.format(busnm) +


You don't set the timestamp so it's initialized with junk. Here you can set it with get tick

nistath · 2019-05-02T04:39:58Z

generator/computers_c.py

                fw('\tCANlib_update_can_{}();\n'.format(busnm))
            fw('}\n\n')
+
+            fw('void CANlib_HandleFrame(CAN_Raw_Bus_T raw_bus) {\n')


How is this different than the update? Where can a user of interrupts provide a timestamped frame and have it accounted for?

nistath · 2019-05-02T04:40:35Z

generator/computers_c.py

            fw('\t}\n}\n\n')

+            fw(
+              'CAN_Raw_Bus_T CANlib_GetConceptualBus(CAN_Raw_Bus_T bus) {\n'


What is this used for?

equationcrunchor · 2019-05-11T16:30:12Z

Made changes. Example usage in sensor-node-timestamped-frames branch of MY19.

nistath · 2019-07-24T03:43:00Z

@equationcrunchor please revisit to resolve conflicts and ping me for review?

equationcrunchor · 2019-07-25T01:02:51Z

@nistath Conflicts resolved.

nistath · 2019-07-29T01:37:53Z

generator/computers_c.py

    frame_handler, is_multplxd
 from math import ceil

+raw_bus_to_handle = {"CAN_1": "hcan1", "CAN_2": "hcan2", "CAN_3": "hcan3"}


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

These variables must be inferred from the spec.

nistath · 2019-07-29T01:45:07Z

generator/computers_c.py

-                )
-            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')


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.

nistath · 2019-07-29T01:45:34Z

generator/computers_c.py

-            fw('\t}\n}\n')
+            fw('void CANlib_HandleFrame(TimestampedFrame *ts_frame, time_t stamp, CAN_TypeDef* instance) {\n')
+            if len(computer.participation['name']['can'].subscribe.keys()) > 0: # check if computer receives messages
+                fw('\tts_frame->stamp = stamp;\n')


nistath · 2019-07-29T01:46:55Z

generator/computers_c.py

+            fw('void CANlib_HandleFrame(TimestampedFrame *ts_frame, time_t stamp, CAN_TypeDef* instance) {\n')
+            if len(computer.participation['name']['can'].subscribe.keys()) > 0: # check if computer receives messages
+                fw('\tts_frame->stamp = stamp;\n')
+                fw('\tswitch ((intptr_t)instance) {\n')


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.

nistath · 2019-07-29T01:48:07Z

generator/computers_c.py

-            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')
+            if len(computer.participation['name']['can'].subscribe.keys()) > 0: # check if computer receives messages


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.

nistath · 2019-07-29T02:00:06Z

generator/computers_c.py


+            fw('extern CAN_HandleTypeDef hcan1;\n')
+            fw('extern CAN_HandleTypeDef hcan2;\n')
+            fw('extern CAN_HandleTypeDef hcan3;\n\n')


Not allowed.

nistath · 2019-07-29T02:05:04Z

src/driver.h


 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);


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

nistath · 2019-07-29T02:07:23Z

src/drivers/inc/stm32f4xx.h

+extern "C" {
+#endif // __cplusplus
+
 CANlib_Transmit_Error_T CANlib_TransmitFrame(Frame *frame, CANlib_Bus_T bus);


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

nistath · 2019-07-29T02:13:51Z

src/drivers/src/stm32f4xx.c

@@ -5,6 +5,7 @@
 #include "driver.h"


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

nistath · 2019-07-29T02:14:28Z

generator/computers_c.py

            fw = f.write
+            fw('#include <time.h>\n')
+            fw('#include <stdbool.h>\n')
+            fw('#include <stm32f4xx_hal.h>\n')


Not allowed.

equationcrunchor added 7 commits April 27, 2019 12:21

Timestamped_Frame struct

4629e6f

Generate timestamped structs

20f88aa

Handle timestamped frames

abdaaa7

Fix bugs

235d4c3

Merge branch 'interrupts' into timestamped-frames

cda72b5

CANlib_ReadFrame now checks if frame is valid

e40b75c

CANlib_HandleFrame now uses CANlib_ReadFrame

cf24235

equationcrunchor requested a review from nistath April 27, 2019 19:55

equationcrunchor added 3 commits April 27, 2019 16:38

Fix typos

0069c3e

Check both FIFOs

a11eb1c

Prevent unused warning

fb6cf2e

nistath requested changes May 2, 2019

View reviewed changes

equationcrunchor added 15 commits May 2, 2019 13:10

Merge remote-tracking branch 'origin/master' into timestamped-frames

c60f943

Rename TimestampedFrame

71c463e

Rename timestamp field to stamp

bff9a8b

Change name

3989e45

Change ReadFrame function

d9404a3

Change CANlib_update functions

1377b3e

Change CANlib_HandleFrame

acc0883

HandleFrame returns bool

616a1f5

New readframe functions

dd9d9db

Change HandleFrame

cbd2495

Fix mistakes

0863f19

Change update can functions

ac7ab57

Fix typos in update_can and switch on instance

0cfa2d9

Extern C stuff

827eeef

Fix typo

04a93f5

equationcrunchor requested a review from nistath May 11, 2019 16:28

Fix unused

55267dc

equationcrunchor added 2 commits July 24, 2019 20:44

Add comment

18be57b

Merge branch 'master' into timestamped-frames

f7dc936

nistath requested changes Jul 29, 2019

View reviewed changes

Conversation

equationcrunchor commented Apr 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nistath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

equationcrunchor commented May 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nistath commented Jul 24, 2019

Uh oh!

equationcrunchor commented Jul 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

equationcrunchor commented Apr 27, 2019 •

edited

Loading

equationcrunchor commented May 11, 2019 •

edited

Loading