-
Notifications
You must be signed in to change notification settings - Fork 241
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
(Legacy) develop_FW-866 OpenRadio interface #504
base: develop
Are you sure you want to change the base?
Conversation
- A hybrid transmitter for openmote-b which loops on all available modulations using openradio interface. - A receiver which sets the rx settings also using open radio interface
cleaning radio header and c file
… indexes instead of slot durations
Really like the struct based approach for |
bsp/boards/openmote-b/board_info.h
Outdated
// also implementation related (because the time to execute the Tx/Rx command is highly dependent on the radio AND the configuration) | ||
uint8_t PORT_delayTx; | ||
uint8_t PORT_delayRx; | ||
} slot_board_vars_t; //board specific slot vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense defining this in bsp/boards/board.h
? you could have something
like this as a default config:
typedef struct {
uint8_t SLOTDURATION;
uint8_t PORT_TsSlotDuration;
uint8_t PORT_maxTxDataPrepare;
uint8_t PORT_maxRxAckPrepare;
uint8_t PORT_maxRxDataPrepare;
uint8_t PORT_maxTxAckPrepare;
uint8_t PORT_delayTx;
uint8_t PORT_delayRx;
} slot_board_vars_t;
/* This would be a default configuration */
#ifndef 10MS_SLOTDURATION
#define 10MS_SLOTDURATION 20 /* in miliseconds */
#endif
#ifndef 10MS_PORT_TsSlotDuration
#define 10MS_PORT_TsSlotDuration 655 /* 655 ticks at @32768Hz */
#endif
#ifndef 10MS_PORT_maxTxDataPrepare
#define 10MS_PORT_maxTxDataPrepare 110 /* 3355us (not measured) */
#endif
#ifndef 10MS_PORT_maxRxAckPrepare
#define 10MS_PORT_maxRxAckPrepare 20 /* 610us (not measured) */
#endif
#ifndef 10MS_PORT_maxRxDataPrepare
#define 10MS_PORT_maxRxDataPrepare 33 /* 1000us (not measured) */
#endif
#ifndef 10MS_PORT_maxTxAckPrepare
#define 10MS_PORT_maxTxAckPrepare 50 /* 1525us (not measured) */
#endif
#ifndef 10MS_PORT_delayTx
#define 10MS_PORT_delayTx 10 /* 549us (not measured) */
#endif
#ifndef 10MS_PORT_delayRx
#define 10MS_PORT_delayRx 0 /* 0us (can not measure) */
#endif
#define SLOT_10_MS_BOARD_CONFIG_DEFAULT { .SLOTDURATION = 10MS_SLOTDURATION, \
.PORT_TsSlotDuration = 10MS_PORT_TsSlotDuration, \
.PORT_maxTxDataPrepare = 10MS_PORT_maxTxDataPrepare, \
.PORT_maxRxAckPrepare = 10MS_PORT_maxRxAckPrepare, \
.PORT_maxRxDataPrepare = 10MS_PORT_maxRxDataPrepare, \
.PORT_maxTxAckPrepare = 10MS_PORT_maxTxAckPrepare, \
.PORT_delayTx = 10MS_PORT_delayTx, \
.PORT_delayRx = 10MS_PORT_delayRx, \
}
static const slot_board_vars_t slot_10ms_board_vars[] =
{
#ifdef SLOT_10MS_BOARD_VARS
SLOT_10MS_BOARD_VARS,
#else
SLOT_10MS_BOARD_VARS_DEFAULT,
#endif
...
/* that for all slots */
...
#ifdef SLOT_40MS_OFDM1MCS0_3_SUBGHZ_BOARD_VARS
SLOT_40MS_OFDM1MCS0_3_SUBGHZ_BOARD_VARS,
#else
SLOT_40MS_OFDM1MCS0_3_SUBGHZ_BOARD_VARS_DEFAULT,
#endif
};
And then per board_info
you would keep the original defines and do the following:
#define SLOT_10MS_BOARD_VARS { .SLOTDURATION = 10MS_SLOTDURATION, \
.PORT_TsSlotDuration =10MS_ PORT_TsSlotDuration, \
.PORT_maxTxDataPrepare = 10MS_PORT_maxTxDataPrepare, \
.PORT_maxRxAckPrepare =10MS_ PORT_maxRxAckPrepare, \
.PORT_maxRxDataPrepare = 10MS_PORT_maxRxDataPrepare, \
.PORT_maxTxAckPrepare = 10MS_PORT_maxTxAckPrepare, \
.PORT_delayTx = 10MS_PORT_delayTx, \
.PORT_delayRx = 10MS_PORT_delayRx, \
}
I think this would also make the migration easier and the init function could
simply be:
void board_init_slot_vars(const slot_board_config_t *slot_board_config) {
slot_board_vars = *slot_board_config;
}
The nice thing is this change could be split from this PR, and since currently only one SLOTDURATION
is possible there would be no need to change any define per BOARD_INFO
just add:
```c
#define SLOT_BOARD_VARS { .SLOTDURATION = SLOTDURATION, \
.PORT_TsSlotDuration =PORT_TsSlotDuration, \
.PORT_maxTxDataPrepare = PORT_maxTxDataPrepare, \
.PORT_maxRxAckPrepare = PORT_maxRxAckPrepare, \
.PORT_maxRxDataPrepare = PORT_maxRxDataPrepare, \
.PORT_maxTxAckPrepare = PORT_maxTxAckPrepare, \
.PORT_delayTx = PORT_delayTx, \
.PORT_delayRx = PORT_delayRx, \
}
…w works correctly :) - moved time_type_t to board.h because this enum seems fundamental to the firmware (even lower than opentimers where it used to be define). It is currently used in a new function in board.c: board_getSlotDuration (time_type_t) - MACRO PRE_CALL_TIMER_WINDOW in opentimers.h now refers to function call keeping track of the current slot board_getSlotDuration(TIME_TICS) instead of the old constant PORT_TsSlotDuration - Added a double radio pin toggle in ri4 and ri5 to indicate the actual start and end of frame at LA for accurate measurement of synchronization offset - both sixtop.c and icmpv6rpl.c now refer to function call keeping track of the current slot board_getSlotDuration(TIME_MS) instead of the old constant: SLOTDURATION
…ant with the new radio.h interface Updated board-specific definitions to be compliant to the new 802.154.e dynamic slot/radio selection (and to the new board.h). This was completed and compiled locally (using armgcc scons compilers) with the following (travis) commands: - scons board=openmote-b-24ghz toolchain=armgcc oos_openwsn - scons board=openmote-cc2538 toolchain=armgcc oos_openwsn - scons board=openmote-cc2538 toolchain=armgcc oos_sniffer - scons board=openmote-cc2538 toolchain=armgcc oos_macpong - scons board=samr21_xpro toolchain=armgcc oos_openwsn - scons board=python toolchain=gcc oos_openwsn - scons board=iot-lab_A8-M3 toolchain=armgcc oos_macpong - scons board=iot-lab_A8-M3 toolchain=armgcc oos_openwsn - scons board=iot-lab_M3 toolchain=armgcc oos_macpong - scons board=iot-lab_M3 toolchain=armgcc oos_openwsn - scons board=openmotestm toolchain=armgcc oos_macpong - scons board=openmotestm toolchain=armgcc oos_openwsn - scons board=openmote-b toolchain=armgcc oos_openwsn
Hello @twatteyne , @changtengfei , and @TimothyClaeys The PR is now ready for review. To test it, you can use SCons or IAR. Make sure you set the following settings:
|
General comments:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't reviewed every file, but the comments in the file I did review are important enough that they require you to rework the code, and restart a new review cycle.
@@ -1,6 +1,9 @@ | |||
Debug | |||
settings | |||
*.dep | |||
*.ewd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good
// also implementation related (because the time to execute the Tx/Rx command is highly dependent on the radio AND the configuration) | ||
uint16_t PORT_delayTx; | ||
uint16_t PORT_delayRx; | ||
} slot_board_vars_t; //board specific slot vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this is the timeslot template. Suggest renaming to timeslot_template_t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh no, the timelost template is defined in the ieee154e.h. This struct is strictly for board specific parameters (regardless of which 154e timeslot template is used)
SLOT_40ms_FSK_SUBGHZ, | ||
SLOT_40ms_OFDM1MCS0_3_SUBGHZ, | ||
MAX_SLOT_TYPES, | ||
} slotType_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why the slot duration is in the name. Just have 3 timeslot templates, for the 3 radio settings we care about.
// available slot templates
typedef enum{
TIMESLOT_TEMPLATE_OQPSK_24GHZ,
TIMESLOT_TEMPLATE_2FSK_SUBGHZ,
TIMESLOT_TEMPLATE_OFDM_SUBGHZ,
TIMESLOT_TEMPLATE_MAX
} timeslot_templates_t;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a 10ms slot template and 20ms slot template. Thoe former is for python the latter has been classically used for 2.4GHz).
I didn't touch them because they are out of my scope (I can still rempve them, or give them different names if you wish?)
#include <source/interrupt.h> | ||
#include <source/sys_ctrl.h> | ||
|
||
\brief Definition of the Open Radio interface for openmote-b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cannot remove authors, add you name at the bottom of the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact this a completely new file for the openradio interface, it is just named the same. The original radio.c file containing cc2538 radio implementation is now this file https://github.com/minarady1/openwsn-fw/blob/develop_FW-866/bsp/chips/cc2538rf/radio_cc2538rf.c
typedef void (*radio_txNow_cb_t)(void); | ||
typedef void (*radio_rxEnable_cb_t)(void); | ||
typedef void (*radio_rxNow_cb_t)(void); | ||
typedef void (*radio_getReceivedFrame_cb_t)( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
bsp/boards/openmote-b/radio.c
Outdated
typedef struct { | ||
radio_reset_cb_t radio_reset; | ||
radio_init_cb_t radio_init; | ||
radio_setConfig_cb_t radio_setConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
#define BOOTLOADER_BACKDOOR_DISABLE 0xEFFFFFFF | ||
#define BOOTLOADER_BACKDOOR_ENABLE 0xF6FFFFFF // ENABLED: PORT A, PIN 6, LOW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PA6 is the pin controlling the bootloader backdoor. We probably hadn't figure out the correct configuration at the time we create the openmote-b port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments. Some suggestions:
- for the if/else condition, always use bracket even there is only one line
- the .gitignore somehow doesn't work, some unnecessary files are still committed.
#define BOOTLOADER_BACKDOOR_DISABLE 0xEFFFFFFF | ||
#define BOOTLOADER_BACKDOOR_ENABLE 0xF6FFFFFF // ENABLED: PORT A, PIN 6, LOW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PA6 is the pin controlling the bootloader backdoor. We probably hadn't figure out the correct configuration at the time we create the openmote-b port.
@@ -1,119 +1,63 @@ | |||
/** | |||
\brief This program shows the use of the "radio" bsp module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove the description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file still for bsp_radio_rx project?
@@ -1,16 +1,13 @@ | |||
/** | |||
\brief This program shows the use of the "radio" bsp module. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep the newline between paragraphs
@@ -0,0 +1,2891 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
somehow the .gitignore doesn't work. The file supposes to be not committed.
…d_getSlotTemplate() into slot_board_vars_t board_selectSlotTemplate(slotType)
Initial commit of code compiling and working and synching but not fully communicating