PX4-Autopilot icon indicating copy to clipboard operation
PX4-Autopilot copied to clipboard

HRT common code ( and derivatives like Tunes library implementation HRT uses)

Open davids5 opened this issue 8 years ago • 8 comments

Now that there are 4 chip architectures stm32, stm32F7, Kinetis and samv7

There is a need to isolate the stm32 arch specific code from the arch agnostic code for drivers (hrt in this case) and derivatives. Such as Tunes library implementation .

davids5 avatar Oct 09 '17 19:10 davids5

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 30 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

PX4BuildBot avatar Feb 21 '18 13:02 PX4BuildBot

This still an issue.

simonegu avatar Mar 06 '18 21:03 simonegu

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[bot] avatar Jan 29 '19 00:01 stale[bot]

We need a pass on the htr to do some clean up and document all the interrupt states in the call tree. then build the unified driver with arch specifics like the tone_alarm.

fyi: @mcsauder see https://github.com/PX4/Firmware/pull/11351 for the 32 bit timer version.

I think this needs a critical section

hrt_call_delay(struct hrt_call *entry, hrt_abstime delay)
{
	entry->deadline = hrt_absolute_time() + delay;
}

There is also this that makes no sense

/**
 * Store the absolute time in an interrupt-safe fashion
 */
hrt_abstime
hrt_store_absolute_time(volatile hrt_abstime *now)
{
	irqstate_t flags = px4_enter_critical_section();

	hrt_abstime ts = hrt_absolute_time();

	px4_leave_critical_section(flags);

	return ts;
}

@LorenzMeier, @bkueng, @dagar how would you feel about changing the new hrt_elapsed_time_atomic to be done differently or removed because hrt_absolute_time already has has a critical section. then: hrt_absolute_time(void) should become _hrt_absolute_time(const volatile hrt_abstime *offset) hrt_absolute_time(void) wraps and calls _hrt_absolute_time(nullptr)

Then hrt_elapsed_time_atomic(const volatile hrt_abstime *then) will use _hrt_absolute_time(then) and it its critical section if (my_offset~==nullptr) do the math and return that value.

The additional overhead is 1 pointer null check and one register load for the call with null ptr and for the call with an offset it is just the math as opposed to 2 cs prologues and epilogues which is many more instructions that add not value.

davids5 avatar Jan 31 '19 12:01 davids5

@davids5 given that hrt_elapsed_time_atomic is almost never used, I don't think it's worth optimizing. On the other hand hrt_absolute_time is used all over the place, called at high frequency, and therefore worth optimizing every bit.

bkueng avatar Feb 04 '19 11:02 bkueng

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Oct 08 '19 11:10 stale[bot]

Is it still relevant @davids5 ?

junwoo091400 avatar Jul 27 '22 16:07 junwoo091400

Yes. It is harder to separate it out

davids5 avatar Jul 27 '22 17:07 davids5