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

Timer configuration #5824

Merged
merged 3 commits into from May 6, 2018

Conversation

Projects
None yet
2 participants
@blckmn
Copy link
Member

blckmn commented May 5, 2018

Dependent on #5825

This puts the ability to modify the timers similar to the resource command.

Timers are configured like so:

# timer
# examples: 
timer A01 1
timer A01 none (or 0)

To list available timers for a given pin:

timer A01 list

The setting is merely an index into the records within the timerHardware array for the target.

A01 above is the pin that the timer is being allocated to.

@blckmn

This comment has been minimized.

Copy link
Member Author

blckmn commented May 5, 2018

Please note that I closed the old PR as this is a different approach.

@blckmn blckmn requested review from mikeller and jflyper May 5, 2018

@blckmn blckmn added this to the Betaflight v3.4 milestone May 5, 2018

@blckmn blckmn self-assigned this May 5, 2018

@blckmn blckmn force-pushed the blckmn:timer_config branch 3 times, most recently from 9553e8f to e96399b May 5, 2018

uint8_t currentIndex = 1;
if (USABLE_TIMER_CHANNEL_COUNT > 0)
{
for (unsigned i = 0; i < USABLE_TIMER_CHANNEL_COUNT; i++) {

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

This fails to build for SITL due to USABLE_TIMER_CHANNEL_COUNT == 0.

uint8_t timerIndex = timerIndexByTag(ioTag);
uint8_t index = 1;
if (USABLE_TIMER_CHANNEL_COUNT > 0)
{

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

Newline.

ioTag_t timerioTagGetByUsage(timerUsageFlag_e usageFlag, uint8_t index)
{
uint8_t currentIndex = 1;
if (USABLE_TIMER_CHANNEL_COUNT > 0)

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

Newline.

@@ -3922,6 +4032,9 @@ const clicmd_t cmdTable[] = {
#ifdef USE_RESOURCE_MGMT
CLI_COMMAND_DEF("resource", "show/set resources", NULL, cliResource),
CLI_COMMAND_DEF("dma", "list dma utilisation", NULL, cliDma),
#ifdef USE_TIMER_MGMT
CLI_COMMAND_DEF("timer", "show timer configuration", NULL, cliTimer),

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

The commands should be in the list in alphabetical order.

@@ -78,6 +78,7 @@
#define PG_SERVO_CONFIG 52
#define PG_IBUS_TELEMETRY_CONFIG 53 // CF 1.x
//#define PG_VTX_CONFIG 54 // CF 1.x
#define PG_TIMER_IO_CONFIG 55 // used to store the index for timer use in timerHardware array in target.c

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

Not sure if iNav wants to adopt this. Should maybe go into the Betaflight range.

@@ -136,4 +136,7 @@
#define TARGET_IO_PORTD (BIT(2))

#define USABLE_TIMER_CHANNEL_COUNT 9
#define USED_TIMERS ( TIM_N(2) | TIM_N(3) | TIM_N(5) | TIM_N(8) | TIM_N(9) )
#define PPM_PIN PC7

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

I'd rather we did not reintroduce usage of these defines, considering they will have to be removed again when moving to universal targets.


const timerHardware_t timerHardware[USABLE_TIMER_CHANNEL_COUNT] = {

DEF_TIM(TIM12, CH2, PB15, TIM_USE_PWM | TIM_USE_PPM, 0, 0),

This comment has been minimized.

@mikeller

mikeller May 5, 2018

Member

Wouldn't it be better to not define any pins, so that a diff reflected the actual target config?

This comment has been minimized.

@blckmn

blckmn May 5, 2018

Author Member

This would become the entire list of available pins (with timers). Ultimately the TIM_USE would go... and it would simply be tag and index needed to find the timerHardware entry.

The timerhardware list will become the ultimate "lookup" of all possible combinations for timers.

This comment has been minimized.

@blckmn

blckmn May 5, 2018

Author Member

Note that this is the start of the timerHardware full mapping. I'd expect others as they add pin requirements to map those timers into the generic targets as options.

This comment has been minimized.

@mikeller

mikeller May 6, 2018

Member

But timerHardware currently assigns one timer to each pin - this may or may not be the one that is used for a particular board, making a board's diff based config look inconsistent because it has to re-define timers for some pins, but not for others. Would it work to use placeholders to indicate 'undefined timer' in the unified targets?

This comment has been minimized.

@blckmn

blckmn May 6, 2018

Author Member

No this allows you to have multiple entries per pin in the array. I have added some for the STM32F7X2 target as an example.

This comment has been minimized.

@mikeller

mikeller May 6, 2018

Member

Ah I see. So the plan is to add all of the possible pin / timer combinations in here? This could probably be auto-generated as a one-off.

This comment has been minimized.

@blckmn

blckmn May 6, 2018

Author Member

It could be auto generated :) or people can add them as needed :)

This comment has been minimized.

@mikeller

mikeller May 6, 2018

Member

I'd prefer to have them auto-generated to start with. Adding them as needed whenever a new universal target is added or an existing one is converted will create a lot of churn, with the potential for bugs to be introduced and for the configuration for other targets to be invalidated, and the end result is that sooner or later all possible combinations will be in here anyway.

This comment has been minimized.

@blckmn

blckmn May 6, 2018

Author Member

I don't think there is an "auto generated" option. It's someone sitting down and going through the datasheet, and ensuring they are all covered.

This comment has been minimized.

@mikeller

mikeller May 6, 2018

Member

I thought they were all listed in timer_def.h, and could be extracted / converted from there?

But no matter how we do it, I think it should be done before target maintainers are starting to convert their targets.

@blckmn blckmn force-pushed the blckmn:timer_config branch 2 times, most recently from db074ee to 3252e86 May 5, 2018

@blckmn blckmn force-pushed the blckmn:timer_config branch from e3d6948 to 93bb409 May 5, 2018

@blckmn blckmn force-pushed the blckmn:timer_config branch from 93bb409 to 6d33fc6 May 6, 2018

@mikeller mikeller merged commit 7174441 into betaflight:master May 6, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.