-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[sd logger] Version 2.0 release, direct SPI logging, UART download #1392
Conversation
Looks really cool! Does it work with regular (==non-ChibiOS) Lisa M/MX board? |
@podhrmic I use it without ChibiOS on a Lisa/S. It is designed to work on Lisa M/MX as well (but not tested). |
Great. I ll test it on Lisa MX |
Hi @bartslinger - sadly it doesn't work on Lisa MX. I have the SD card connected at SPI1, I am using a Quad_Lisa_MX example with the logger module, and it never writes anything on the card (no matter what position is the radio switch). Any ideas? |
@podhrmic It does not work with the logger module. The card needs to be connected directly to the spi port like this: |
@podhrmic Could you try if the LED goes on when you flip the switch? For the Lisa/S, I had to disable another LED in the board makefile. Then, in your airframe config it should be something like this
|
I am using SD card breakout from Sparkfun: https://www.sparkfun.com/products/544 |
Ok that should be fine. Please note that it does not use a filesystem like FAT, so you won't be able to see the logs if you insert the card in the computer directly. An SD Logger settings tab should also appear in the ground station. Could you check which value sdcard1.status and sdlogger_spi.status have? |
ah I see. So in that case how do I download the data? Do I just plug the autopilot into USB to get the ACM device? |
Tested with different LED and different radio configuration. The radio SWITCH inputs gets through, but never triggers the logging or the LED. Any ideas? |
The sdcard1.status = 1 means an error in (the connection to) the sd-card, probably already during initialization. I have not yet built in a way to see where it fails exactly, but can add this relatively easily. At 15 different locations in sdcard_spi.c the The LED only goes ON if the logging has actually started. That explains why it doesn't turn on. Downloading can be done by connecting the boards UART port to the computer with FTDI or BlackMagicProbe. |
I now added a setting to see where the error occurred. Let me know what it says. |
Without having looked at this in detail yet, it sounds like a really great modue/addition! |
@flixr You're absolutely right about that. Is there another way to get those values on request? |
Yep, still the same error. I have the connections as: Using a scope I can't see any data coming through, nor even a toggle on CS or SCK. Are the pins properly defined on Lisa MX? |
A dedicated telemetry message? |
Do you have the following defines in var/generated//ap_srcs.list?
|
Yep, a dedicated telemetry message (or using the DEBUG message) is the other option... @podhrmic do you use the correct SPI slave (SS pin), check against the lisa/mx board file. |
OK, I have good news - adding these two lines into airframe config fixed the logging:
So now the whole config for Lisa MX looks like this:
(I am using Taranis SBUS radio). The LED lights up when I flip the switch, so that looks good. Now, how do I download the data? I know there is the UART tool, but how do I use it? Thanks! |
You need to run sdlogger_download in sw/logalizer. The screenshots in the first post show an example. Make sure you use the same uart as is used for telemetry. That is currently the only supported method. |
I am getting a seg fault:
(the ac_id and the port are definitely correct) |
Damn, should have done that one test-driven as well. I will try to find and fix that, although its hard to debug since I don't have this problem. |
Correct, if it can find a GPS message in the log, |
Would be great if you could add some more explanation/docs to the module xml file. |
|
||
#include "std.h" | ||
#include "mcu_periph/link_device.h" | ||
#include "subsystems/radio_control.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only link_device and spi includes actually seem to be needed in this header.
Better to only include the reset in the .c file where they are actually needed.
Would be nice if this wouldn't depend on the radio control subsystem and make this optional (and protected with |
<init fun="sdlogger_spi_direct_init()"/> | ||
<periodic fun="sdlogger_spi_direct_periodic()" freq="512" start="sdlogger_spi_direct_start()" stop="sdlogger_spi_direct_stop()" autorun="TRUE"/> | ||
<datalink message="SETTING" fun="sdlogger_spi_direct_command()"/> | ||
<makefile> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would make sense to set the target to ap
here and merge the other makefile section into it, otherwise it will try to build the module for nps, sim, etc as well and fail....
Can you please explain the need for changing |
uint8_t buffer[SDLOGGER_BUFFER_SIZE]; | ||
uint8_t idx; | ||
uint32_t log_len; | ||
uint8_t command; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these commands?
@bartslinger since this is very useful, would be great if we could merge it ASAP. And it would be great if you could explain why you added an empty mode... why is that needed? |
Relevant Wiki documentation: https://wiki.paparazziuav.org/wiki/SDLogger_SPI_Direct to be extended over time |
It would be great to have it merged. Then we have +4 students using PPRZ master who will test it in real life and can make improvements, give feedback and add documentation and examples. |
Improvement and fixes to be done after pulled into master:
|
Ok my priorities have shifted a bit, but I can spend some time on this tomorrow. |
@bartslinger , not that you need to fix the issues alone, it's more that if in master those are the point to improve soon afterwards. I will be of assistance and finish some task for that. For now fixing the most important remarks of @flixr after the merge. IMHO, yes plz let's merge it, we have commitment of multiple to improve soon after merge. |
IMHO there is no point if merging an unfinished feature - the pull request can be tested as is, improved and then finally merged in master. I would like to use it too, these are just my 2 cents. |
start/stop functionalityThe initialization sequence is called on startup of the system. The initialization involves periods of waiting which is implemented by using periodic function. Therefore, the module is started from the beginning. I would have preferred RTOS for this since you then need to call initialization only once. It is possible to rebuild it such that the card initialization happens when the first logs starts, but that would delay the first measurement. However, that would allow usage of start/stop functions of the module. I'm not planning to implement this in this version. Empty messageThe downloading of data is implemented by putting all logged data on the uart bus. I only want to save logger messages, so therefore I disabled other messages with empty telemetry config. If you have other suggestions to disable all messages except from the sdcard, let me know. command / settingsThe |
@bartslinger thanks a lot for fixing the target, cleaning up and the clarification! Regarding the periodic function: Regarding the empty periodic messages mode: If the downloader can correctly handle some other messages in between now and then, I think this is good to merge. If not, we should address this. |
Actually, I don't know, because the data is processed by So it's possible to have multiple start/stop/periodic functions per module? How? |
If that is a problem, then one way to deal with this would be to add a specific message for sending log data and only parse this message in the downloader on the ground... while that would incur some more overhead, I think it would be the way to go... You can add multiple periodic functions in the module xml and assign them different start/stop functions: |
Maybe the mistake was to use the same start byte for pprz and logger format, therefore we can't correctly split them. What a about a |
So if this is a problem, I guess it will some more work which should be addressed in a separate pull request. |
yes, agreed |
This module provides a tool for high-speed logging where weight- and size-limitations play part. It only requires a micro-SD card which is soldered directly to the SPI-bus, for example on a lisa/s. The logger is activated by flipping a switch on the transmitter. Up to 42 logs can be recorded during one session (battery kept connected). The data remains on the card when the battery is disconnected, so that you can replace the battery before downloading data to the computer. Previous logs will be overwritten as soon as the first log is initiated after boot. The messages to be logged can be selected in the telemetry config file, under the Logger process. Next to that, it is possible to call pprz_msg_send_xxx from anywhere in the code, using a reference to the sdlogger. See also http://wiki.paparazziuav.org/wiki/SDLogger_SPI_Direct
This module provides a tool for high-speed logging where weight- and size-limitations play part.
It only requires a micro-SD card which is soldered directly to the SPI-bus, for example on a lisa/s.
The logger is activated by flipping a switch on the transmitter. Up to 42 logs can be recorded during one session (battery kept connected). The data remains on the card when the battery is disconnected, so that you can replace the battery before downloading data to the computer. Previous logs will be overwritten as soon as the first log is initiated after boot.
The messages to be logged can be selected in the telemetry config file, under the Logger process. Next to that, it is possible to call pprz_msg_send_xxx from anywhere in the code, using a reference to the sdlogger.
Telemetry config example:
A tool is provided to download the data at high speed over UART:
Limitations: For now, it can only be used with uart downlink. This is perfect for lisa/s where UART pins are broken out.