p-o-n-y/pony

Navigation software framework/core application structure in ANSI C

pony

Navigation software framework/core application structure in ANSI C

pony's Issues

pony_gnss_gps_sat is exactly the same as pony_gnss_glo_sat

Поскольку количество эфемерид не хардкодится, между структурами pony_gnss_gps_sat и pony_gnss_glo_sat нет разницы.
Всё равно делаем одинаковые структуры, или переименовываем pony_gnss_gps_sat в pony_gnss_sat и используем для всех?

define'ы в pony.h

@a-kozlov-msu

pony/pony.h

Line 7 in 1d0f768

#define PONY_LINAL

Define'ы ведь обычно выносят в код хоста перед подключением библиотеки? PONY_LINAL оставим в pony.h? Или это просто временная мера?

Микрооптимизация (1)

В

pony/pony.c

Lines 364 to 375 in 1d0f768

	if (gps->sat != NULL)
	{
	for (i = 0; i < gps->max_sat_count; i++)
	if (gps->sat[i].eph != NULL)
	{
	free(gps->sat[i].eph);
	gps->sat[i].eph = NULL;
	}
	free(gps->sat);
	}
	gps->sat = NULL;

и

pony/pony.c

Lines 433 to 444 in 1d0f768

	if (glo->sat != NULL)
	{
	for (i = 0; i < glo->max_sat_count; i++)
	if (glo->sat[i].eph != NULL)
	{
	free(glo->sat[i].eph);
	glo->sat[i].eph = NULL;
	}
	free(glo->sat);
	}
	glo->sat = NULL;

финальное приравнивание NULL'у нужно внести в конец блока if, так как в случае, если gps->sat или glo->sat равны NULL, им всё равно присваивается NULL.

bug report

pony.h line 205 is not needed as separators have already been skipped, instead of them the first relevant symbol of comfiguration is skipped.

for (i = 0; confstr[i] && i < conflen; i++) {
			// skip all non-printable characters, blank spaces and commas between groups
			for (; confstr[i] && (confstr[i] <= ' ' || confstr[i] == ',') && i < conflen; i++); // already skipped here
			// if no group started at this point
			if (confstr[i] != '{')
				break;
			// if a group started
			else {
				group_layer = 1;
				while (group_layer > 0 && confstr[i] && i < conflen) {
					i++;
					if (confstr[i] == '{')
						group_layer++;
					if (confstr[i] == '}')
						group_layer--;
				}
			}
		}
// skip all non-printable characters, blank spaces and commas between groups
for (i++; confstr[i] && (confstr[i] <= ' ' || confstr[i] == ',') && i < conflen; i++); // line 205
// start from this point
*groupptr = confstr + i;

pony_strncmpeff, pony_locatesubstrn, pony_locatesubstreff

pony_strncmpeff, pony_locatesubstrn и pony_locatesubstreff сейчас никак не используются. Всё равно пока оставляем их?

Clear up the functions' parameters

'+' character when parsing numerics from configuration strings

В строке

pony/pony.c

Line 613 in 733724f

if ((confstr[0] - '0' > 9 || confstr[0] - '0' < 0) && confstr[0] != '-')

и ей подобных символ '+' должен быть допустимым наравне с '-' и другими

Suspended plugins don't release resourses

В примере main лишняя точка с запятой

github.com/p-o-n-y/pony/wiki/main

if (pony_init( "строка конфигурации" ));

free() и NULL в функциях pony_free_*

На сейчас в функциях освобождения частей pony есть 3 варианта освобождения самих частей:

.
1

pony/pony.c

Lines 274 to 275 in 1d0f768

	free(pony.imu);
	pony.imu = NULL;

В самой же функции imu освобождается и ссылка приравнивается NULL'у.
imu есть один, доступ через пони (возможно лучше сделать через параметр функции)

.
2

pony/pony.c

Line 506 in 1d0f768

void pony_free_gnss(pony_gnss *gnss)

В самой же функции с gnss'ом ничего не делается, это необходимо, так как gnss'ы лежат динамическим массивом, который потом освобождается разом, и общей ссылке присваивается NULL.
gnss'ов много, доступ к текущему через параметр функции

.
3

pony/pony.c

Line 378 in 1d0f768

free(gps);

glo - аналогично
В самой же функции gps освобождается, но не приравнивается NULL'у, NULL'у приравнивается во внешней функции. Это разделение выглядит не очень логично, т.к. после освобождения памяти приравнивание NULL'у по идее не может навредить утечками памяти и т.д.
gps'ов много, но в рамках одного gnss'а один, доступ к текущему через параметр функции

.

Итак,
во-первых, третий вариант лучше убрать (привести к первому или второму), так как он неоправданно множит стандарты, что не очень
во-вторых, возможно первый вариант лучше привести ко второму, на случай, если внезапно нужно будет делать несколько imu (ну мало ли)

Add links to wikipedia pages where needed/helpful

Comments

@a-kozlov-msu
Комментарии на уже не русском языке

pony/pony.h

Line 21 in 44b89b3

typedef struct //äëÿ èçìåðåíèé, âûðàæàþùèõñÿ ÷åðåç îäíî ÷èñëî

и серый код

pony/pony.c

Line 260 in 44b89b3

void pony_format(char* str)

убираю?

Константы gps

1. Константы для gps одинаковые, но у каждого gnss свои, то есть под одно и то же при нескольких gnss'ах память на одинаковые вещи выделяться несколько раз. Можно решить, скажем, переносом структуры из gnss'а в pony_bus.
   

2. Присваивание значений происходит даже при отсутствии gps. Возможно тогда стоит перенести структуру с константами в gps? Или наоборот, сделать её общей (переназвать в gnss_const)?
   

3. При n gnss'ов значения констант присваиваютя n*(n+1)/2 раз. Это можно решить переносом строки из pony_init_gnss
   
   

   pony/pony.c

   Line 465 in 1d0f768

   pony_init_gnss_gps_const();

   
   
   на строку 677 в pony_init.
   Или использованием одной общей структуры.

pony_terminate working incorrectly

Right now pony_terminate makes plugins run in finish mode twice

bug - empty but labeled groups in configuration

in pony.c line 254

return (*grouplen)>0 ? 1 : 0;

treats empty but labeled groups in configuration as being not found

add dynamic memory allocation checks

После каждого динамического выделения памяти должна быть проверка на нулевой указатель (NULL).

Функции ядра pony, в которых происходит выделение памяти, должны возвращать значение char 0 или 1, которое должно проверяться вызывающей функцией.

Это касается pony_init, pony_setDASize и возможно других.

Уточнение комментариев (1)

pony/pony.h

Lines 148 to 152 in 1d0f768

	char* cfg; // GNSS configuration string pointer
	int cfglength; // configuration string length
	char* settings_cfg; // pointer to a part of GNSS configuration common to all systems
	int settings_length; // full configuration string length

Наверное описания длин стоит поменять местами, иначе непонятно.

Кстати, общие подчасти строк конфигурации мы теперь называем settings, или это только в этом случае?

"conf" to "cfg"

На сейчас переименовано только в частях шины, нужно ли переименовывать в методах тоже, для большей читаемости?

Пример кода плагина можно улучшить

github.com/p-o-n-y/pony/wiki/plugins

plugin заменить на plugin_name

Инициализацию логично делать в начале функции, это соответствует "обычному" программированию, удобнее переделывать существующие функции в плагин.

// список используемых переменных
я бы вообще убрал, это очевидно, тем более это не просто список, а декларация

Clear up the configuration string format, especially for gnss

Are galileo and beidou supported?

Если они поддерживаются, было ли это начиная с e81e758 для gal и с 5baff31 для bds?

Обнуление ссылок на подстроки конфигурации

Раз уж после освобождения ссылок на части пони (imu и т.д.) им присваивается NULL, стоит ли так же обнулять ссылки на подстроки конфигурации и их длины в функциях free?

move pony internal service functions declaration to pony.c

Все определения функций pony, за исключением тспользуемых хостом add_plugin, init, step и terminate нужно перенести в pony.c, так как использовать их предполагается только там.

Возможно, что какие-то мы перенесём обратно для использования юзером или добавим в будущем (например предполагается библиотека функций для работы с матрицами), но сейчас это внутренние функции pony, которые не должны появляться в коде хоста при включении pony.h.

When did glonass actually become supported?

Это был не 1b328c8? Или позже?

Whad does "single-receiver gnss support to the bus" mean?

В данный момент в вики пишу историю изменений, в основном направленную на разработчиков плагинов, поэтому стараюсь в ней описывать новые возможности для них без того, чтобы им пришлось самим разбираться в коммитах. Так что эти вопросы будут поступать ещё некоторое время. Когда история изменений будет в приличном состоянии, добавлю ссылку на неё в readme.

Данный вопрос про летний major update, а именно что значит пункт added single-receiver gnss support to the bus.

bug report

pony.c line 210
Нужно добавить && (*groupptr)[*grouplen] != '}' к условиям, так как поиск "def" используя "gnss:" в "{gnss: {gps: abc} def} ghi" вернёт длину, соответствующую "def} ghi"

		// determine the length, counting until the end of the string or when a group started
for (; (*groupptr)[*grouplen] && (*groupptr)[*grouplen] != '{'; (*grouplen)++);

Configuration outside groups

@a-kozlov-msu

pony/pony.h

Line 175 in 3d74440

char* cfg; // part of configuration that is outside of any group

Точно стоит выносить внешнюю часть конфигурации в core? Если я правильно понял, там ведь лежат неиспользуемые юзером переменные. Или по стандарту хостом внешняя часть конфигурации не используется?

Some configuration strings don't give expected results

For example "{imu:}{gnss:{gps:}{bds:}}"

freeing memory of *obs in pony_gnss_gps_sat

Так как определяется во время выполнения, ответственность за освобождение памяти несёт пользователь?