-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
UAVCAN external mag use #2912
Comments
Should be fixed by #2914 |
Ok, issue isn't related to timestamping it seems. I'm beginning to thing that Brian's test on #2818 and mine were somehow false positives and it worked because of boot timing artifacts. Or mayhe this is a a regression.
|
EDIT : Nevermind. I added some debugging (and formatting fixes :P ) to the voting class, the prios are OK. UAVCAN is higher than internal.
Still don't get why:
|
@LorenzMeier Any ideas on the above? I'm trying to debug it in any case. |
Things seem to be quite messed up. How many magnetometers do we have onboard anyway? :O Booting without UAVCAN gives me this directly :
And what the hell do we have here?
When a sensor failsafe is triggerred, it seems that we have a memory(?) issue as well. No NSH commands work:
|
Um, you're running out of memory. I'm going to try to reproduce it tomorrow again, with the timestamping fix. |
Booted with UAVCAN sensors disconnected:
So far so good. Connecting Zubax GNSS while PX4 is running:
Wat? Ok, rebooting PX4 with Zubax GNSS still connected:
The phantom magnetometer disappeared again. This is what
Where did the third magnetometer come from? |
75 is ORB_PRIO_DEFAULT, so something is not going right when those are advertised or evaluated. |
Please re-try with master. I've improved the failover logic, in particular for cases where a higher priority sensor becomes available later. |
Is the fw server running? Could there be 2 node ids, one at boot, then switched to a new one based on the uuId byte ordering or parameter stored fo node Id? |
David, no, since the bootloader doesn't publish sensor msgs
|
Doesn't seem to be the case - see my posts from yesterday.
Seems to be unrelated - see below.
Checked, no it doesn't advertise twice. However, I found something weirder that I don't understand. 1. Stack sizeWhen dealing with weird issues I check stack usage, which I did this time. I noticed that 2. Advertisement logicEither it's flawed or I'm missing something. We have this code in Then, the call to Also, Any suggestions? |
You need to untangle the class instance from the ORB publication. They are entirely unrelated. The issue is potentially here:
Has to be a valid pointer: The logic then tries to find the first free uORB group ID and allocates it. It should never try to allocate an existing one. |
They are untangled. I just compared the logic of uORB advertisements in UAVCAN with the HMC5883 driver, seems identical.
It is a valid pointer. So the issue is elsewhere, apparently. |
@LorenzMeier I would appreciate if you could have a look at it. I checked with a debugger that the UAVCAN driver advertises the mag topic only once, yet the attitude estimator sees two sensors. |
@pavel-kirienko Can you try checking the status output when UAVCAN is enabled, but with the cable unplugged? I remember something was weird, which might help us. |
Looks sane to me (cable unplugged):
|
Hmm. Can you try with UAVCAN startup at boot? If it works for now, we can track down the phantom mag later. |
With UAVCAN started at boot, cable unplugged, still one mag:
Yes, doesn't help:
Shall we escalate the issue to uORB or sensor management? |
@pavel-kirienko Does your phantom mag change values, or is it frozen at one value? It seems different but very close to the 2nd mag, so the publication has to come from UAVCAN. |
Yes it does change values. |
@pavel-kirienko #2918 gets the mag into a working state atleast, irrespective of the phantom magnetometer. |
@LorenzMeier it would help a lot if you or someone else familiar with internals of uORB could have a look at this issue (related #3122). It looks like the problem is caused by uORB rather than UAVCAN driver (see my descriptions above). Thanks in advance. |
Could you please repro with current master, just so we're sure we're not chasing a ghost? I will try to come up with a unit test catching this. |
Yep 100% repro. Just tried. |
Thanks. Will dig into this. |
@LorenzMeier @pavel-kirienko It keeps switching between mags 0(internal) and 1 (uavcan) continiously. Some debugging shows
So - our UAVCAN mag confidence keeps dropping to 0 for some reason. I added debugging, which shows that we fail here : 2 and 3 keep happening continuously, with some delay in-between. |
Woohoo. Gotcha - we got the exact same sensor value 100 times in a row. @pavel-kirienko Your game now, I guess. Increasing the threshold to 200 instead of 100 'fixes' it. |
@mhkabir I can't repro this. See the log: ftp://pkir.net/px4_uavcan_mag_samples.txt. I have added this to your branch to print the mag values: diff --git a/src/modules/uavcan/sensors/mag.cpp b/src/modules/uavcan/sensors/mag.cpp
index e184ff1..d2a3926 100644
--- a/src/modules/uavcan/sensors/mag.cpp
+++ b/src/modules/uavcan/sensors/mag.cpp
@@ -160,5 +160,6 @@ void UavcanMagnetometerBridge::mag_sub_cb(const uavcan::ReceivedDataStructure<ua
_report.z = (msg.magnetic_field_ga[2] - _scale.z_offset) * _scale.z_scale;
unlock();
+ DEVICE_LOG("V %.6f %.6f %.6f", double(_report.x), double(_report.y), double(_report.z));
publish(msg.getSrcNodeID().get(), &_report);
} It can be seen in the log that the sensor values do not get anywhere close to 100 successive identical values prior to failure. |
I put in debug printfs here:
Whenever the switching was occurring, I can clearly see the validator dropping the confidence to zero, trigerring the first printf, and then the second one. Please see if you can repro this from inside the validator. It may be so that the sensors app (since it paces the |
This is almost completely fixed by :
These, once done gives us complete and working UAVCAN sensor support (related : #3158) just in time for the v1.1.0 release :) The apparent uORB bug still remains which causes a phantom magnetometer to be reported, but this does not affect performance. |
I'm closing this for now. Looks like we need to set up our own vehicle with UAVCAN to debug this. |
The compass auto-switching seemed to be working on the delta angles pull without any timing hacks, but now again only the internal one is being used. This needs to be vetted and fixed.
The UAVCAN mag gets instance ID 1 currently, but has a higher prio, so should be used instead of the internal.
@pavel-kirienko Some help on debugging this appreciated.
The text was updated successfully, but these errors were encountered: