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

Develop fw 708 2sfk50 40msTS #444

Open
wants to merge 39 commits into
base: develop_FW-708
Choose a base branch
from

Conversation

jmmunoz86
Copy link
Contributor

this provides the timeslot timing constants for the 2-FSK 50 kbps PHY.
TS is 40 ms long.

Copy link
Member

@changtengfei changtengfei left a comment

Choose a reason for hiding this comment

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

I left comments in the PR, please revise them.

@@ -1164,7 +1172,7 @@ port_INLINE void activity_ti1ORri1(void) {
}

port_INLINE void activity_ti2(void) {

debugpins_frame_toggle();
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't toggle this pin here.

@@ -1209,6 +1217,7 @@ port_INLINE void activity_ti2(void) {
ieee154e_vars.radioOnThisSlot=TRUE;
// change state
changeState(S_TXDATAREADY);
debugpins_frame_toggle();
Copy link
Member

Choose a reason for hiding this comment

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

same here

//static const uint8_t chTemplate_default[] = {
// 5,6,12,7,15,4,14,11,8,0,1,2,13,3,9,10
//};

static const uint8_t chTemplate_default[] = {
Copy link
Member

Choose a reason for hiding this comment

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

This channelhopping sequence should move to radio_24ghz/radio_subghz.c file

#define NUM_CHANNELS 16 // number of channels to channel hop on
#define DEFAULT_CH_SPACING 1200 // default channel spacing for subghz
#define DEFAULT_FREQUENCY_CENTER 863625 // defualt freque
#define NUM_CHANNELS 3 // number of channels to channel hop on
Copy link
Member

Choose a reason for hiding this comment

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

num of channels should move to radio part as well

#define DEFAULT_CH_SPACING 1200 // default channel spacing for subghz
#define DEFAULT_FREQUENCY_CENTER 863625 // defualt freque
#define NUM_CHANNELS 3 // number of channels to channel hop on
#define DEFAULT_CH_SPACING 200 // default channel spacing for subghz
Copy link
Member

Choose a reason for hiding this comment

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

this variable could be change if using different phy layers for subghz

#ifdef SLOTDURATION_10MS
wdAckDuration = 80, // 2400us (measured 1000us)
#else
wdAckDuration = 98, // 3000us (measured 1000us)
wdAckDuration = 260, // 5400us using 50 kbps
Copy link
Member

Choose a reason for hiding this comment

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

This timing variable and above are related to the PHY layer used. So they are no longer belong to IEEE802.15.4E.h file. Define those variables in radio.h files, those variables will be set according which PHY layers are used.

By fixing the length of packet to transmit:

  • For wdDataDuration wdAckDuration, delayTx, delayRx timing variables, they are related to the sending rate of each PHY layer.
  • For other timing but slotDuration, they are related the speed to access radio chip
  • For slotduration, it's determined both sending rate and the speed to access radio chip. So decide it at last.

@@ -17,7 +17,7 @@

The superframe reappears over time and can be arbitrarily long.
*/
#define SLOTFRAME_LENGTH 101 //should be 101
#define SLOTFRAME_LENGTH 31 //should be 101
Copy link
Member

Choose a reason for hiding this comment

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

Does it have to be 31?

@@ -9,9 +9,9 @@
<debug>1</debug>
<settings>
<name>C-STAT</name>
<archiveVersion>260</archiveVersion>
<archiveVersion>261</archiveVersion>
Copy link
Member

Choose a reason for hiding this comment

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

ewt file is not needed to be committed, please delete it from repository.

@@ -631,7 +631,7 @@ owerror_t sixtop_send_internal(
// msg->l2_radioType = (radioType_t)(msg->l2_dsn&0x01);
// }

msg->l2_radioType = 0;
msg->l2_radioType = 1;
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 a hack for debugging purpose, you can do this for now., It's better to think what the right way to determine which radioType to use.

#endif
#define PORT_delayRx 0 // 0us (can not measure)
// radio watchdog
#else // 40 ms
Copy link
Member

Choose a reason for hiding this comment

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

The timing below depends on which radio is used by openmote-b, those timing should be moved to radio.h files can call dynamically.

I recommend to use a fixed timing for FSK modulation, and use the timing variable for 2.4GHz and OQPSK 800kb/s as well. Only delayTx timing need to be adjust to each three PHY layers. With this, the timing part would be much easier.

radiotype defines PHYs whereas modem either sub-ghz or 2.4 ghz.
Copy link
Member

@changtengfei changtengfei left a comment

Choose a reason for hiding this comment

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

Please don't add new changes after the review. I left some comments, please just fix them, no more new changes added.

#define PORT_maxRxDataPrepare 30 // 1007us (measured 84us)
#define PORT_maxTxAckPrepare 40 // 305us (measured 219us)
// radio speed related
#define delayTx_2FSK_50 67
Copy link
Member

Choose a reason for hiding this comment

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

The delayTx , delayRx are defined in radio_2d4ghz / radio_subghz files

MODEM_2D4GHZ = 0,
MODEM_SUBGHZ = 1,
FREQBAND_ANY = 2
} modem_t;
Copy link
Member

Choose a reason for hiding this comment

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

why create a modem type here? what the difference betwen this and the radioType

@@ -71,7 +71,6 @@ void radio_2d4ghz_setFunctions(radio_functions_t* funcs){
funcs->radio_rfOff_cb = radio_2d4ghz_rfOff;
funcs->radio_setFrequency_cb = radio_2d4ghz_setFrequency;
funcs->radio_change_modulation_cb = radio_2d4ghz_change_modulation;
funcs->radio_change_size_cb = radio_2d4ghz_change_size;
Copy link
Member

Choose a reason for hiding this comment

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

if this callback function is not used, you need remove it from the openradio.h file as well.

Also the radio_2d4ghz_change_size function should be removed

//=========================== public ==========================================
static void radio_subghz_read_isr(void);
static void radio_subghz_clear_isr(void);
void config_ofdm_1_800_subGHz(void);
Copy link
Member

Choose a reason for hiding this comment

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

function should start with name radio_subghz_

@@ -46,9 +46,15 @@ typedef struct {

radio_subghz_vars_t radio_subghz_vars;

phy_tsch_config_t phy_tsch_config_2fsk_50_subGHz;
Copy link
Member

Choose a reason for hiding this comment

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

are those variables used in this PR?

phy_tsch_config_ofdm_1_800_subGHz.num_channels = 5;

}

Copy link
Member

Choose a reason for hiding this comment

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

Here is initialize the variables, when are they being used?

static const registerSetting_t basic_settings_fsk_option1 []={


static const registerSetting_t basic_settings_fsk_option1_subghz []={
Copy link
Member

Choose a reason for hiding this comment

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

why rename it?

@@ -88,7 +88,7 @@
</option>
<option>
<name>OCLastSavedByProductVersion</name>
<state>8.11.1.13270</state>
<state>8.20.1.14181</state>
Copy link
Member

Choose a reason for hiding this comment

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

the project file changes shouldn't commit

@@ -66,7 +66,7 @@
</option>
<option>
<name>OGLastSavedByProductVersion</name>
<state>8.11.1.13270</state>
<state>8.20.1.14181</state>
Copy link
Member

Choose a reason for hiding this comment

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

the project file changes like version shouldn't commit.

@changtengfei
Copy link
Member

Can one of the admins verify this patch?

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.

2 participants