Toward a centralized, flexible PWM resource coordination subsystem

[rolandash]

Note
This PR is a workable branch, little bit behind latest px4/master (so all checks may fail), but as i imagine it will not be merged in short time so i need to rebase it anyway till then. You are welcome to comments on the new model and design, to see any improvements needed, and co-work if you are also interested.
If anyone would like to have a test flight you can find it here:
https://github.com/airmind/OpenMindPX
as its updated from time to time.

So far it is only tested on MindPX and MindRacer. Please let me know if you found any problem on other hardware (TAP-V1 probably broke on LED drivers, but it was not a correct implementation and needs to be fixed later anyway).

Describe problem solved by the proposed pull request
This PR starts the step towards a centralized while flexible PWM resource management system which introduce many interesting features.

Existing PWM subsystem majorly hand-wire pwm resource allocation like timers, channels, pin numbers, scattered in many different locations such as board configs, airframe config files, mixer files, or source codes, many of them hard-coded. This makes attempts to expand supported PWM devices quite difficult as one needs to manually settle all potential conflicts on resource usage.

This PR wants to build a centralized while flexible way to support as many kind of PWM devices as possible with only few simple runtime configurations needed through some kind of Human-Machine Interface(like from GUI of GCS).

Describe your preferred solution
A new model of PWM devices and a centralized resource management group of classes are defined to
achieve this goal. A detailed description of the model and design can be found here:
Design document

Some interesting features can be brought in by the new design. For example,

1. Camera trigger will not be limited on the first 6pins of main output. Pilot can use any free pins on Main/Aux port, if the first 6 pins all occupied, through few configuration of parameters.
2. A pilot that flys a hexa airframe, now can use the remaining 2 channels on Main port to drive an external PWM heater to heat any other components(not only the flight controller) on his vehicle, simply by selecting the pin number from GUI of GCS(needs further work).
3. in the meanwhile, he can still connect a set of external PWM LEDs to AUX channels, still through configuration of the pin numbers from GUI of GCS(needs further work).
   He will not need to change the codes and compile to upload everytime he wants to change the task modules layout for different missions.

Additional context
@davids5 It will be very interesting to see the possibility of it to be combined with the hardware manifest work you initiated. I am expecting your co-work on it.
@dagar This PR is somewhat independent of the original PR #7208 and #8142 that separating the FMU from other 'flight management' tasks. I prefer that we pursuing that target in an other PR.
@DonLakeFlyer Ultimately all these configurations has to be done through GCS. I think this will need your help on QGC side, if you like it and are interested on helping it out.
@testflights I will be appreciate if you can help to test the PR. And i strongly recommend you have MindPX/MindRacer hardwares added into your testing package :-)

@mzahana I think you can have all 16 (8 main + 8 aux) channels on MindPX fully funtional and all VTOL frames available now. Pls let me know on any issue. @TSC21

@LorenzMeier

[davids5]

@rolandash this was really needed. Thank you! I will review it in detail next week. Would you be able to join the dev call wednesday 8:00 PM PST and walk us through it?

[rolandash]

@davids5 thank you and that's great!

Are you sure the dev call is 8PM PST? as in dev call calendar it seems to be 8AM PST.
I can do 8PM, but 8AM will be too late for me...

In latter case, I will be happy to discuss it on Slack (need someone invites me in though), or add more comments/documents where you need ...

[davids5]

@davids5 thank you and that's great!

Are you sure the dev call is 8PM PST? as in dev call calendar it seems to be 8AM PST.

My bad 8 AM.

I can do 8PM, but 8AM will be too late for me...

In latter case, I will be happy to discuss it on Slack (need someone invites me in though), or add more comments/documents where you need ...

We can plan a skype call with @dagar when it would work for you. What time zone are you in?

[rolandash]

@davids5 thanks that will be nice.

I am in UTC+8, usually local time 11:00 ~ 21:00 is good for me. I am traveling till Wed next week. So sometime on Thursday or Friday would be great.

Please let me know if that suits your.

[davids5]

@dagar what time will work for you. I will fit it what works for both you and @rolandash

[dagar]

@rolandash could you please schedule something as late as possible for you either Thursday or Friday?
invite: daniel@agar.ca

[DonLakeFlyer]

I looked at the linked design document but that didn't really give me much of an idea as to what needs to happen in QGC either. So will need something I can understand to help out here.

[rolandash]

@dagar will about 23:00 my local time work for you?

[rolandash]

Thanks @DonLakeFlyer You are right that's not complete yet. Will let you know when more details come out.

[davids5]

@dagar will about 23:00 my local time work for you?

@rolandash - wouldn't that be 8AM PST which is when the dev call is? Is that time slot workable?

[rolandash]

@davids5 i was thinking Fri. 23:00, which when i can have some special arrangement that night and next day, and it probably be easier for you guys....

So which one we go for, Thur. 21:00 or Fri. 23:00 ? @dagar

[rolandash]

@davids5 @dagar Lets have the call on 23.00 Fri. my local time, i assume its better time for your guys.

I have added you on Skype @dagar
@davids5 would you please add me ning.roland@hotmail.com ? Would you mind host the call?

[davids5]

@rolandash @dagar - I have added ning.roland@hotmail.com to skype and will place the call 8:00 AM PST on 9/28.

[dagar]

[davids5]

1. FMU breakup:
   Safety px4fmu split safety button into new module #10119

2. Multi module https://github.com/PX4/Firmware/tree/pr-module_base_multi

3. Document the io timer hierarch
   Trigger
   Servo
   Capture

4. Make timers instanceable at run time.

5. CPP class them

[rolandash]

@dagar the ECU

https://github.com/PX4/Firmware/blob/ea0941c24fb208184bf7feeeaae4648f5099f5aa/src/drivers/px4ecu/px4ecu.cpp

[rolandash]

@davids5
I found out why i was hesitate on 'rgb_pwm_led' during the call.

1. MindPX has 8 main + 8 aux
2. the new Mindracer, has 8 main + 5 aux (exposed), and the remaining 3 pwm channels are used for rgb_pwm_led (the super bright one onboard).

The problem is we want to use one single target (firmware) for both hardwares(another purpose of this PR, for the release of new mindracer hardware). This brings some extra issues in:

1. the channels aux port using has overlap with pwm led channels, it may interfere each other. Its not like in V5, as channels for pwm led are independent from other devices.
2. the rate that pwm led using has constrains on what rate aux port channels can use on new Mindracer hardware.
3. Of course, we still want rgb_pwm_led driver can also work properly on MindPX, where someone may want to use it at a different rate.

So these actually challenges on:

1. can a pwm device share part of its channels with another device to maximum resource util?
2. can a pwm device select the most available rate to work with the resources available instead of wasting them?

These explains where the ideas of channel conflict/rate conflict checking coming from. Sorry i forgot these during the call as they were buried under many design threads back few months ago, but it is important.

This is actually a quite typical example what the problems we will encounter when we try to add a new pwm device into existing PX4 system, not limited to pwm led only.

Instance-able FMU/ECU, and timers are great and we should work on that, but may be not enough to solve the problems, e.g., we can make main & aux run on main processor, but a 6-outputs aux mixer which works on MindPX, may break the pwm led on new Mindracer. And these are what i tried to address in PwmGroups class in this PR.
Part of the implementation needs to be in UI side, but in flight controller side there will be structures needed to support the concept.

Hope you will take this into account when we move forward. @dagar

This is the link of MindPX/new Mindracer pinout map

Please let me know if you need further discussions.

[rolandash]

Another question about if a pwm device can use channel across the boarder of main and aux port, on another thought it probably be feasible based on current design(depends on how we revise some existing limits), though pilots may not intend to do that by intuition. But technically its an option we can turn it on or off.

[davids5]

@rolandash @dagar - here is what we have
io_timer-asbuilt.pdf

The led pwm and pwm_trigger are more or less copies. The drv_pwm_servo and drv_input_capture are lend themselves to hierarchical model. led pwm and pwm_trigger may be absorbed by having a pointer based implementation.

[rolandash]

@rolandash @dagar - here is what we have
io_timer-asbuilt.pdf

The led pwm and pwm_trigger are more or less copies. The drv_pwm_servo and drv_input_capture are lend themselves to hierarchical model. led pwm and pwm_trigger may be absorbed by having a pointer based implementation.

@davids5 thanks. thats is a clear picture.
A question is can you explain bit more about what you mean by a pointer based implementation?

[davids5]

@rolandash

I have to look at the use case in more detail, but in general:

https://github.com/PX4/Firmware/blob/master/src/drivers/stm32/drv_io_timer.c#L523-L537

would become int io_timer_init_timer(io_timer_dev *priv, unsigned timer)
and all the compile time io_timers[timer] would become priv->io_timers[timer]

Then the same code can work on multiple sets of io_timer and timer_io_channels described by instances of io_timer_dev.

Then the 0th instance of the FMU (code) would use the 0th instance of io_timer_dev.
the 1st instance of the FMU (code) would use the 1st instance of io_timer_dev. etc.

Ideally this would all be done in C++ and the specialization hinted at in the uml would be implemented. The challenges is that the PX4IO uses the same code and we have more resource limitations there.

@dagar when you get a chance we should spend some time hashing this out.

[rolandash]

@rolandash

I have to look at the use case in more detail, but in general:

https://github.com/PX4/Firmware/blob/master/src/drivers/stm32/drv_io_timer.c#L523-L537

would become int io_timer_init_timer(io_timer_dev *priv, unsigned timer)
and all the compile time io_timers[timer] would become priv->io_timers[timer]

Then the same code can work on multiple sets of io_timer and timer_io_channels described by instances of io_timer_dev.

Then the 0th instance of the FMU (code) would use the 0th instance of io_timer_dev.
the 1st instance of the FMU (code) would use the 1st instance of io_timer_dev. etc.

Ideally this would all be done in C++ and the specialization hinted at in the uml would be implemented. The challenges is that the PX4IO uses the same code and we have more resource limitations there.

@dagar when you get a chance we should spend some time hashing this out.

@davids5
thanks for the explanation. I roughly got the idea. A few comments:

1. I guess the intention is to use drv_io_timer.c as a centralized coordinator of timer/channel resources. The good side of this method is we already have io_timer_channel_mode_t here so it would be natural to add resource allocation code here. The down side is other detailed resource management like channel map under different PWM working modes / mixer channels / shared channels etc., will inflate this driver into too complex than a low level driver should be.
2. My suggestion is while io_timer_channel_mode_t is a good place to add logic into but still keep drv_io_timer as simple as possible, and have another management class on top of it to handle higher level resource mgmt. I am not aware the detailed design of io_timer_dev yet, hope that structure can balance the design in some way like this.

We can set up another discussion if necessary @dagar

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

Closing as stale.