Skip to content
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

Increasing index telemetry message #2501

Open
fvantienen opened this issue Mar 16, 2020 · 12 comments
Open

Increasing index telemetry message #2501

fvantienen opened this issue Mar 16, 2020 · 12 comments

Comments

@fvantienen
Copy link
Member

Messages with increasing index which are used in for example both the Flight Recorder and Telemetry will both increase the index. This could lead to missing information in one of the targets for the telemetry message.
Example messages: ESC, UART_ERRORS, I2C_ERRORS etc..

I think this should be solved with a separate counter per telemetry. What do you think @gautierhattenberger ?

@gautierhattenberger
Copy link
Member

Are you talking about this index: https://github.com/paparazzi/paparazzi/blob/master/sw/airborne/mcu_periph/uart.c#L224 ?
How would you implement this ?

@fvantienen
Copy link
Member Author

yes that index. For this message it's not a problem for me but especially for the ESC message I have this problem.

I just came up with a possible solution. Would it be possible to get the amount of processes in the telemetry file in the generator? Such that each of them ahs an unique number which they can give to the callback function inside the periodic_telemetry_send_....... functions?
This all should be done in gen_periodic.ml

@fvantienen
Copy link
Member Author

fvantienen commented Mar 16, 2020

Then if you have the total number inside these functions you can make a list of numbers which you can count so it would change to:

uint8_t uart_nb_cnt[PERIODIC_PROCESS_NB];
...
uart_nb_cnt[process_id]++;
  if (uart_nb_cnt[process_id] == 9) {
    uart_nb_cnt[process_id] = 0;
  }

Would you have time to change the ocaml part of this @gautierhattenberger ? Then I can take a look at the airborne code.

@gautierhattenberger
Copy link
Member

see branch gen_telemetry_process_number

@fvantienen
Copy link
Member Author

fvantienen commented May 1, 2020

Thank you!
One smalle request can you fix this line to a correct ID instead of 0: ea658b7#diff-8569302ad9668c0e2c3250673a443ff3L183

(Line 183: let p_id = ref 0 in)

@fvantienen
Copy link
Member Author

I see what is going wrong there probably. The p_id should be defined above the for loop over the processes.

@gautierhattenberger
Copy link
Member

I fixed it, thanks. I guess we are lucky nobody was using this...

@OpenUAS
Copy link
Contributor

OpenUAS commented Sep 16, 2020

If all is sane what prevents it take to honor this pull request? If it needs more testing, we can test and then confirm OK or not.

@fvantienen
Copy link
Member Author

fvantienen commented May 6, 2021

I see two options to solve this problem:

  • Change the callback function inside register_periodic_telemetry(telemetry_cb) throughout the whole project to include the process_id (with __attribute__((unused)) for most messages)
  • Add a new register_periodic_telemetry_ext which can register a different callback and then add a type to the telemetry slots with an union of two different type of callbacks

Which one do you prefer @gautierhattenberger ? Or do you have another good solution?

@fvantienen
Copy link
Member Author

I would prefer option 1 and then add a struct as attribute which we could extend later on of more things needs to be passed through. Then add the trans and dev also to this struct, similar to pprzlink v2.

@gautierhattenberger
Copy link
Member

@fvantienen Fabien and I where actually thinking about changing things regarding the telemetry registration. Our idea was to declare that in the module XML, like the declaration of datalink event callback. As a result, all register calls would be generated and included at the end of the modules init.
The reason is that it allows to optimize the messages sequence in the end, as we can match the requested messages (from telemetry file) and the one actually used (from loaded modules). It will also reduce the size of the structure used to register the callbacks to what is actually needed.

If we do that, I think it is a good idea to also change the callback prototype to have a single structure containing the trans and device pointers, as well as all necessary information we might need (and allow to add more in the future).
I'm currently working on an other matter, so I can't provide a date for these changes on telemetry system. But if possible, I think it is worth waiting to do the job only once.

What's your opinion on this possible solution ?

@fvantienen
Copy link
Member Author

That seems like a good solution, I don't want this feature right away but it was kind of bugging me for a while now. So I will wait for your solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants