PIC32 i MQTT: Problemy z biblioteką przy błędnym logowaniu

Mon Aug 08, 2022 7:14 pm   

Wróciłem ostatnio do jednego ze swoich starych projektów, zrealizowanych
w oparciu o PIC32. Chciałem dodać do niego jedną dość ważną
funkcjonalność - możliwość wrzucania danych za pomocą MQTT.
Udało mi się znaleźć bibliotekę napisaną w oparciu o starzy stos
Microchipa (biblioteki MLA), który użyłem w swoim projekcie. Zależało mi
na tym, bo nie chciałem przenosić całego kodu na nowsze biblioteki Harmony.

https://github.com/dominicusplatus/mqttx

Potrzeba było trochę eksperymentów, jednak w końcu udało mi się
uruchomić bibliotekę. Dość szybko okazało się, że daleko jej do ideału.
Działa całkiem nieźle, jeśli serwer ma wyłączoną autoryzację, albo w
pakiecie CONNECT wysyłamy prawidłowy login i hasło.
Problemy zaczynają się w momencie, gdy próbujemy podać nieprawidłowe
dane do logowania. Sprawdziłem komunikację Wiresharkiem - serwer
odpowiada prawidłowo, jednak klient zupełnie to ignoruje, przystępując
do wysyłania komunikatu. Po paru podejściach powoduje to wykrzaczenie
programu i reset mikrokontrolera.

Przyjrzałem się bliżej kodowi i znalazłem przyczynę. Podczas parsowania
pakietu CONNACK wywoływana jest funkcja MQTTReadPacket(), która pobiera
dane z socketa do bufora MQTTBuffer. Potem jednak kod sprawdza stan stan
bajtu rxBF[1], gdzie spodziewa się znaleźć Return Code i na jego
podstawie podjąć decyzję do co dalszego działania.

Problem polega na tym, że w obecnej wersji kodu tablica rxBF nie jest
nigdzie wykorzystywana. Nie trafiają do niej żadne dane (a więc również
i pakiet CONNACK) i cały czas znajdują się w niej same zera. Program
interpretuje więc odczytaną wartość jako Connection Accepted i
przechodzi do dalszych czynności.

Chciałem się bliżej przyjrzeć funkcji MQTTReadPacket() i tutaj trafiłem
do króliczej nory. ;)

Okazuje się, że została ona zdefiniowana i zdeklarowana z pustą listą
parametrów (bez void), podobnie jak to się robi z bezparametrowymi
funkcjami w C++. Widzę jednak, że w paru miejscach w kodzie funkcja
przyjmuje parametr w postaci wskaźnika na BYTE, przykładowo:

BYTE llen;
WORD len= MQTTReadPacket(&llen);

Potem zawartość takiej zmiennej jest wykorzystywana w kodzie jako
element indeksu tablicy MQTTBuffer - również w tych częściach kodu,
które działały prawidłowo. Szybkie poszukiwania w internecie ujawniły,
że możliwość zdeklarowania pustej listy argumentów to historyczna
zaszłość. Wszyscy przestrzegają przed robieniem tego. Natomiast nigdzie
nie mogę znaleźć informacji o tym, w jaki sposób to działa i co
właściwie robią te kawałki kodu. Ktoś ma jakiś pomysł?

Mon Aug 08, 2022 9:00 pm   

On 08/08/2022 19:14, Atlantis wrote:

Podziel się większym kontekstem tego kodu.

Mon Aug 08, 2022 10:00 pm   

Atlantis <marekw1986NOSPAM@wp.pl> wrote:

Nie możesz po prostu poprawić tego kodu.
Ja, implementujące mqtt, po prostu powyginalem jakaś niedorobioną
bibliotekę i będzie git. Przynajmniej w zakresie w jakim tego
potrzebowalem.

jp

Mon Aug 08, 2022 11:43 pm   

On 08.08.2022 21:00, heby wrote:


W poprzedniej wiadomości był link do repozytorium na GitHubie. Tam jest
całość.

Tue Aug 09, 2022 7:13 am   

Tue Aug 09, 2022 8:36 am   

Tue Aug 09, 2022 9:47 am   

Atlantis <marekw1986NOSPAM@wp.pl> napisał(a):

Odwrotnie. Nie jest inicjalizowana bo nie jest statyczna. Inicjalizacja
domyślna zmiennych odbywa się raz, przed uruchomieniem main(). Zmienne
automatyczne, alokowane na stosie, mają de facto przypadkowe wartości.

--
Grzegorz Niemirowski
https://www.grzegorz.net/

Tue Aug 09, 2022 10:16 am   

Tue Aug 09, 2022 10:29 am   

On 09.08.2022 09:47, Grzegorz Niemirowski wrote:


Masz rację, moja pomyłka.
W ramach eksperymentu usunąłem z kodu wszelkie przypadki zmiennych
przekazywanych do MQTTReadPacket() przez wskaźnik na pustej liście
parametrów. Takich sytuacji było dosłownie tylko kilka.
Najbardziej istotna wydawała się sytuacja, gdzie taka zmienna llen była
potem wykorzystywana do wyliczania indeksów tablicy MQTTBuffer. Usunąłem
wszelkie odwołania do tej zmiennej, ustawiłem parametr MQTTReadPacket na
void i skompilowałem program. Wszystko działa tak jak poprzednio - ani
lepiej, ani gorzej.
Wcześniej założyłem, że musi się tam dziać jakaś dziwna "ezoteryczna
magia" wynikająca ze specyfiki języka i w jakiś niewidoczny w kodzie
sposób odpowiednia wartość trafia jednak do zmiennej llen i wszystko
działa jak powinno.
Być może jednak po prostu w moim przypadki ten "if" nie jest nigdy
wywoływany, więc program nie wchodził nigdy w tę gałąź i losowe wartości
ze stosu trafiające do llen nie dawały o sobie znać.

Tue Aug 09, 2022 10:53 am   

Tue Aug 09, 2022 7:13 pm   

Tue Aug 09, 2022 8:38 pm   

On 09.08.2022 19:13, J.F wrote:


To jest właśnie jakaś zaszłość historyczna, o której sam wcześniej nie
miałem pojęcia. W C++ funkcja z pustą listą parametrów równa się funkcji
z parametrem void. Jednak w C działa to inaczej - pusta lista parametrów
oznacza niesprecyzowaną liczbę parametrów. Do takiej funkcji możemy
przekazać albo nic, albo cokolwiek i taki kod się skompiluje, jednak z
wnętrza funkcji do tych parametrów i tak nie będziemy mieć dostępu.
Liczyłem na to, że może jakaś ezoteryka języka pozwala na niejawne
przekazanie czegoś w ten sposób jednak wychodzi na to, że ktoś po prostu
zmieniał tę funkcję i po prostu usunął parametry zamiast przerobić
funkcję na void.



Dane powinny trafić do MQTTBuffer, jednak z tego co widzę funkcja
MQTTReadPacket() w jej obecnej formie nie radzi sobie z wiadomością
CONNACK. Odczytuje tylko dwa pierwsze bajty (nagłówek) ale już nie dwa
kolejne, z czego nas interesuje ostatni - result code.



Na to wygląda. Udało mi się skontaktować z autorem wcześniejszej wersji
tej biblioteki na PIC32. To znaczy wcześniejszej w stosunku do tego, co
sam znalazłem na GitHubie. A i on prawdopodobnie opierał się na jakimś
wcześniejszym kodzie. Przesłał mi jakąś swoją starą wersję kodu -
nietestowaną i jak się okazało niedziałającą przez kilka bugów (m.in.
powodujących zatrzaskiwanie się maszyny stanów albo błędy podczas
czytania z socketa). Co jednak istotne, w tej wersji kodu funkcja
MQTTReadPacket przyjmowała jeden parametr - wskaźnik na BYTE. Wychodzi
się na to, że pusta lista parametrów powstała, gdy ktoś później
przepisywał ten kod.

Wychodzi na to, że trzeba będzie teraz przyjrzeć się tej funkcji i
ustalić, czemu nie parsuje prawidłowo CONNACT.



Rozróżnienie na len i llen bierze się ze specyfiki MQTT. W podstawowych
wiadomościach (np. kontrolnych) mamy niewielką wartość len. Jednak gdy
trzeba np. załączyć większy payload, załącza się jeszcze dodatkową
informację. Podejrzewam, że w takich sytuacjach obydwie wartości mogą
być potrzebne do manipulowania do manipulowania buforem. Z drugiej
strony obecna wersja MQTTReadPacket posiada lokalną, statyczną tablice o
nazwie lengthLength, która może być właśnie do tego wykorzystywana.

Wed Aug 10, 2022 12:06 am   

Sprawa ciągle nie daje mi spokoju, wiec przyjrzałem się jeszcze raz
trochę bliżej działaniu funkcji MQTTReadPacket(). Nadal nie wiem jak
rozwiązać problem, ale sprawa przynajmniej się trochę wyjaśniła.

W uproszczeniu korzystanie z tej funkcji wygląda następująco:

case MQTT_CONNECT_ACK
WORD len = MQTTReadPacket();
if (len >= 2) {
//Tutaj wykonują się główne operacje
//Tylko tutaj może zostać zmieniony stan maszyny stanów
}
break;

Funkcja może być wywołana w tym miejscu wielokrotnie i dalej przejdziemy
dopiero wtedy, gdy zwróci ona dwa lub więcej (wartość ta oznacza liczbę
bajtów zapisanych do bufora). Sama funkcja MQTTReadPacket również
posiada swoją własną maszynę stanów, której wartość jest zapisywana w
lokalnej zmiennej statycznej. Od stanu tej maszyny zależy co funkcja zwróci.

Dopisałem do kodu kilka printf-ów, żeby zobaczyć co się tam dzieje.
Wynik wygląda następująco:

Tutaj wywolujemy MQTTReadPacket() w MQTT_CONNECT_ACK
m_state=0 <- stan po wywołaniu funkcji w MQTT_CONNECT_ACK
len=2 <- wartość zwrócona

Tutaj wywołania w innych częściach programu
m_state=2
m_state=3
m_state=0

Tutaj wywolujemy MQTTReadPacket() w MQTT_CONNECT_ACK
m_state=2 <- stan po wywołaniu funkcji w MQTT_CONNECT_ACK
len=0 <- wartość zwrócona

Tutaj wywolujemy MQTTReadPacket() w MQTT_CONNECT_ACK
m_state=3 <- stan po wywołaniu funkcji w MQTT_CONNECT_ACK
len=0 <- wartość zwrócona

Tutaj wywolujemy MQTTReadPacket() w MQTT_CONNECT_ACK
m_state=0 <- stan po wywołaniu funkcji w MQTT_CONNECT_ACK
len=2 <- wartość zwrócona

Tutaj wywołania w innych częściach programu
m_state=2
m_state=3
m_state=0


Jak widać maszyna stanów przy kolejnych wywołaniach funkcji
MQTTReadPacket() posiada wartości 0, 2 albo 3.
Jeśli funkcja zostanie wywołana gdy jej maszyna będzie w stanie 0, to
zwrócona zostanie wartość 2 - spowoduje to, że będziemy mogli wejść do
ifa i wykonać operacje przewidziane dla MQTT_CONNECT_ACK.
Jeśli jednak w momencie wywołania funkcji jej maszyna stanów będzie
ustawiona na 2 albo 3, zwrócone zostanie zero. If się nie wywoła, a wiec
nie opuścimy MQTT_CONNECT_ACK i wrócimy do tego samego miejsca w
kolejnym obiegu pętli.

Co więcej - wywołania MQTTReadPacket w innych miejscach programu również
biorą udział w tym cyklu.

Kolejny wniosek jest taki, że "magiczne liczby" w switch/case wewnątrz
MQTTReadPacket to tak naprawdę kolejne etapy pracy maszyny stanów tej
funkcji.

Więc główny problem polega teraz na tym, żeby w tym miejscu nie było
zwracane 2, ale 4. Do MQTTBuffer powinny trafiać cztery bajty, a
ostatnio z nich będzie kodem wyniku, przesyłanym w pakiecie CONNACK.
Konieczna będzie dokładniejsza analiza tego, co właściwie ta funkcja
robi.

Wed Aug 10, 2022 12:13 am   

On 09.08.2022 19:13, J.F wrote:
[...]

standardy. Od C11 (aka ISO/IEC 9899:2011) takie deklaracje zostały
uznane za przestarzałe (chociaż nadal dopuszczalne).

Tu masz to opisane razem z cytatami ze standardu/ów:

https://stackoverflow.com/questions/13950642/why-does-a-function-with-no-parameters-compared-to-the-actual-function-definiti

https://stackoverflow.com/questions/18378993/c-and-defining-a-function-prototype-with-no-parameters

Wed Aug 10, 2022 9:52 am   

On Wed, 10 Aug 2022 00:13:41 +0200, JDX wrote:

Jest to moze i jakis błąd, ale jest szerszy - C pozwala na wywołanie
funkcji z dowolną listą parametrow, bo inaczej printf nie moglby
dzialac.

Co prawda powinno to zostac zdefinioniowane z uzyciem "..."
.... no to moze i jest jakis błąd kompilatora, ze nie zgłasza rożnic
w prototypie, deklaracji i wywolaniach ...

J.