Support SkyViper V2450 #7539

tridge · 2018-01-15T02:36:16Z

This adds full support for the SkyViper 2450. It incorporates driver changes (some of which have separate PRs), plus additions to ArduCopter, new temperature calibration code and new declination code.
To build this firmware use:
./waf configure --board skyviper-v2450
it builds under ChibiOS

rmackay9 · 2018-01-15T23:18:36Z

Looks like it's failing travis because "degrees C" appears in the parameter descriptions..

davidbuzz · 2018-01-17T01:31:39Z

libraries/AP_Compass/Compass_PerMotor.cpp

+    // @DisplayName: Compass per-motor4 Z
+    // @Description: Compensation for Z axis of motor4
+    // @User: Advanced
+    AP_GROUPINFO("4",  6, Compass_PerMotor, compensation[3], 0),


compass compensation per-motor is limited to 4 motors?

yes, for now, happy to extend it if there is demand

davidbuzz · 2018-01-17T01:33:09Z

libraries/AP_Compass/Compass_PerMotor.cpp

+    }
+
+    // this currently assumes first 4 channels. 
+    uint16_t pwm = hal.rcout->read_last_sent(motor_map[motor]);


assume is a lot like "TODO", is this safe-in-practice?

davidbuzz · 2018-01-17T01:35:32Z

libraries/AP_Compass/Compass_PerMotor.h

+class Compass;
+
+// per-motor compass compensation class. Currently tied to quadcopters
+// only, and single magnetometer


maybe raise this known limitation as an issue for future revisit ?

worth discussing on a dev call

davidbuzz · 2018-01-17T01:53:36Z

libraries/AP_Radio/AP_Radio.cpp

+}
+
+// set the 2.4GHz wifi channel used by companion computer, so it can be avoided
+void AP_Radio::set_wifi_channel(uint8_t channel)


this is more of a "avoid_wifi_channel" or "set_wifi_channel_to_avoid" than "set_wifi_channel", causual observer/s might think this function uses the wifi channel.

davidbuzz · 2018-01-17T01:59:30Z

libraries/AP_Radio/AP_Radio_backend.h

+    virtual void start_recv_bind(void) = 0;
+
+    // return time in microseconds of last received R/C packet
+    virtual uint32_t last_recv_us(void) = 0;


A uint32 counting microseconds will be able to count up to about 35 minutes. It's common for people to power up some of their equipment but not all of it (such as while waiting for gps lock, or making coffee). Is this going to potentially wrap-around if the TX is left turned off for more than ~35minutes.. ?

davidbuzz · 2018-01-17T02:02:25Z

libraries/AP_Radio/AP_Radio_cc2500.cpp

+    }
+#endif
+
+    if (now - last_pps_ms > 1000) {


hardcoded threshold of 1000 , maybe a #define, or a parameter?

davidbuzz · 2018-01-17T02:03:32Z

libraries/AP_Radio/AP_Radio_cc2500.cpp

+
+// handle a data96 mavlink packet for fw upload
+void AP_Radio_cc2500::handle_data_packet(mavlink_channel_t chan, const mavlink_data96_t &m)
+{


not impl, clearly.

davidbuzz · 2018-01-17T02:11:05Z

libraries/AP_Radio/AP_Radio_cc2500.cpp

+    bindTxId[1] = info.bindTxId[1];
+    bindOffset = info.bindOffset;
+    listLength = info.listLength;
+    memcpy(bindHopData, info.bindHopData, sizeof(bindHopData));


After blindly memcpy()-ing a chunk of data from ( possiby unreliable) storage into this struct, is it worth defensively checking to make sure that the data is 'good' before we trust it?

davidbuzz · 2018-01-17T02:18:35Z

libraries/AP_Radio/AP_Radio_cypress.cpp

+                     sizeof(_irq_handler_wa),
+                     TIMEOUT_PRIORITY,        /* Initial priority.    */
+                     irq_handler_thd,  /* Thread function.     */
+                     NULL);                     /* Thread parameter.    */


I thought the point of the 'hal' was to hide this sort of OS specific thing from the ArduPilot code. Maybe this chibios "virtual timer" chVTObjectInit and 'Thread' chThdCreateStatic activities should be put in a hal->timer() or hal->thread() or something less platform specific?

yes indeed - and Sid has some patches to add APIs for this

davidbuzz · 2018-01-17T02:24:10Z

libraries/AP_Radio/AP_Radio_cypress.cpp

+        dsm.num_channels = MAX(dsm.num_channels, ch);
+    }
+
+    if (now - last_debug_print_ms > 1000) {


another hardcoded 1000 value, see earlier comment on a #define or parameter?

davidbuzz · 2018-01-17T02:28:37Z

libraries/AP_Radio/AP_Radio_cypress.cpp

+    stm32_gpiosetevent(CYRF_IRQ_INPUT, true, false, false, irq_radio_trampoline);
+#elif CONFIG_HAL_BOARD == HAL_BOARD_CHIBIOS
+    hal.gpio->attach_interrupt(HAL_GPIO_RADIO_IRQ, trigger_irq_radio_event, HAL_GPIO_INTERRUPT_RISING);
+#endif


this is another one of those cases where a hal should mean this block of #if code wouldn't be necerssary.
Can't hal->attach_interrupt(....) be made smart enough to know if it's on px4, and then call stm32_gpiosetevent behind the scenes?

davidbuzz · 2018-01-17T03:04:19Z

libraries/AP_Radio/AP_Radio_cypress.cpp

+    hrt_call_after(&wait_call, 10000, (hrt_callout)irq_timeout_trampoline, nullptr);
+#elif CONFIG_HAL_BOARD == HAL_BOARD_CHIBIOS
+    //Note: please review Frequency in chconf.h to ensure the range of wait
+    chVTSet(&timeout_vt, MS2ST(10), trigger_timeout_event, nullptr);


can these #ifdefs be moved into the hal-> ?

davidbuzz · 2018-01-17T03:06:30Z

I have reviewd, and put in a bunch of inline comments. but it's a +1 from me here for merge.

WickedShell · 2018-01-18T10:36:16Z

@tridge It looks like this include the per-motor compass stuff which is already a it's own PR #6448 and has open review comments that don't seem to be addressed in this PR either. Just tagging it that there is some review that was done from a different location on that part of this.

rmackay9 · 2018-01-27T03:11:50Z

I know it's slightly bad behaviour but I plucked out the flight mode channel change into master. I did this because a user was pinging me about it so it gave me the opportunity to say, "it's done!".
8967448

Sadly, I slightly modified the commit to remove the addition of the #define for TOY_MODE. Sorry, I probably should have left that part in to avoid the merge conflict but I didn't for some reason.

this allows it to be used for a wide range of loop rates

on I2C the lower bus bandwidth changes the tradeoffs

these were unusued but still allocated

needed for good scheduling with I2C IMU

different button mapping for new controller

may be needed on other boards in the future, but for now we only know its needed on an I2C 20789, which is rare

this adds FHLD_* parameters for FlowHold mode. It is a large patch as it needs to disentagle the mode class to enable it to be used in Parameters.h

remove unused peripherals

tridge · 2018-02-08T06:37:01Z

merged after discussion with Randy
many thanks for input today from Peter, Randy and Michael

tridge added the ChibiOS label Jan 15, 2018

tridge requested a review from rmackay9 January 15, 2018 02:36

tridge force-pushed the chibios-skyviper branch from 822e676 to 891fe56 Compare January 16, 2018 03:09

davidbuzz reviewed Jan 17, 2018

View reviewed changes

tridge force-pushed the chibios-skyviper branch 2 times, most recently from 8e414a8 to a04532a Compare January 17, 2018 08:50

tridge force-pushed the chibios-skyviper branch 6 times, most recently from 2864eb6 to 955abde Compare January 20, 2018 06:41

tridge force-pushed the chibios-skyviper branch from 955abde to 8d1c115 Compare January 29, 2018 01:52

tridge force-pushed the chibios-skyviper branch 2 times, most recently from 94a56af to 7748a28 Compare February 6, 2018 23:29

tridge and others added 27 commits February 8, 2018 17:04

AP_Scheduler: scale perf_info with loop rate

98a8ea5

this allows it to be used for a wide range of loop rates

Copter: pass loop rate to perf_info

041b519

Sub: pass loop rate to perf_info

60c6d7b

AP_InertialSensor: added whoami for prototype 20789

7e59d08

AP_InertialSensor: improved handling of I2C invensense sensor

9c1eaa8

on I2C the lower bus bandwidth changes the tradeoffs

HAL_ChibiOS: disable trace buffers

631cf73

these were unusued but still allocated

HAL_ChibiOS: raise I2C bus priority on F412

ed80437

needed for good scheduling with I2C IMU

AP_Radio: fixed startup with stored bind info

8acbb7d

AP_Radio: added SRT packet formats for cc2500

9025b30

AP_Radio: fixed timing of telem packets

305f717

HAL_ChibiOS: fixed F412 OF reset pin

c70fcbe

Frame_Params: fixed orientation on F412

6a761fa

AP_Radio: implement OTA update for cc2500

0d2b544

Copter: enable F412 toymode button setup

8df2819

different button mapping for new controller

Frame_Params: set TMODE_ENABLE=2 for the F412

65f296e

ChibiOS: implement way to clear i2c bus by toggling clock line

89c2d94

HAL_ChibiOS: enable I2C clearing on F412 board only

807a837

may be needed on other boards in the future, but for now we only know its needed on an I2C 20789, which is rare

AP_TempCalibration: fixed build warning

1288877

AP_Radio: fixed build warnings

97e421a

HAL_ChibiOS: fixed printf arguments

cc690ca

Copter: enable parameters inside mode objects

78801f4

this adds FHLD_* parameters for FlowHold mode. It is a large patch as it needs to disentagle the mode class to enable it to be used in Parameters.h

AP_Radio: allow build of AP_Radio under PX4

ff1a4a2

waf: added skyviper-v2450-px4 build

f1519a0

HAL_ChibiOS: fixed build warning

fc1c0cf

HAL_ChibiOS: simplify skyviper-v2450 config

fe4f45d

remove unused peripherals

Copter: fixed units on flowhold brate rate

01ce928

mk: added AP_TempCalibration and AP_Radio

6602198

tridge force-pushed the chibios-skyviper branch from 95dfec9 to 6602198 Compare February 8, 2018 06:04

tridge merged commit 88d53aa into ArduPilot:master Feb 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support SkyViper V2450 #7539

Support SkyViper V2450 #7539

tridge commented Jan 15, 2018

rmackay9 commented Jan 15, 2018

davidbuzz Jan 17, 2018

tridge Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018

tridge Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018 •

edited

tridge Jan 17, 2018

davidbuzz Jan 17, 2018

davidbuzz Jan 17, 2018 •

edited

davidbuzz Jan 17, 2018

davidbuzz commented Jan 17, 2018

WickedShell commented Jan 18, 2018

rmackay9 commented Jan 27, 2018

tridge commented Feb 8, 2018

Support SkyViper V2450 #7539

Support SkyViper V2450 #7539

Conversation

tridge commented Jan 15, 2018

rmackay9 commented Jan 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidbuzz Jan 17, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidbuzz Jan 17, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidbuzz commented Jan 17, 2018

WickedShell commented Jan 18, 2018

rmackay9 commented Jan 27, 2018

tridge commented Feb 8, 2018

davidbuzz Jan 17, 2018 •

edited

davidbuzz Jan 17, 2018 •

edited