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

Fix CI #4

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Fix CI #4

merged 1 commit into from
Mar 18, 2024

Conversation

mathieucarbou
Copy link

@mathieucarbou mathieucarbou commented Mar 15, 2024

This PR activates CI and fix some issues which were preventing the project from compiling.

  • Add workflow_dispatch
  • Fix test which was breaking CI
  • Update CI plugins
  • Only run CI with esp32dev and esp32s3box (only for Lily boards)
  • Declared library to only be compatible with esp32

You can see the build passing in my fork here: https://github.com/mathieucarbou/lewisxhe-TinyGSM/actions

Here are below some comments

@mathieucarbou mathieucarbou marked this pull request as draft March 16, 2024 13:30
@mathieucarbou mathieucarbou force-pushed the master branch 6 times, most recently from d449904 to e3f8a74 Compare March 16, 2024 14:50
- Add workflow_dispatch
- Fix test which was breaking CI
- Update CI plugins
- Only run CI with esp32dev and esp32s3box (only for Lily boards)
- Declared library to only be compatible with esp32
@mathieucarbou mathieucarbou marked this pull request as ready for review March 16, 2024 15:02
Comment on lines +46 to +50
# added in this fork
TINY_GSM_MODEM_A7608,
TINY_GSM_MODEM_A7670,
TINY_GSM_MODEM_SIM7020,
TINY_GSM_MODEM_SIM7672
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added new implementations in CI

@@ -87,5 +97,5 @@ jobs:
echo "${{ env.LIBRARY_INSTALL_SOURCE }}"
pio lib --global install ${{ env.LIBRARY_INSTALL_SOURCE }}
sed -i 's/\/\/ #define TINY_GSM_MODEM_SIM800/#define TINY_GSM_MODEM_SIM800/g' ${{ matrix.example }}/*
platformio ci --project-option='build_flags=-D ${{ env.TINYGSM_MODEM_TO_USE }}' --project-option='framework=arduino' --board=uno --board=leonardo --board=yun --board=megaatmega2560 --board=genuino101 --board=mkr1000USB --board=zero --board=teensy31 --board=bluepill_f103c8 --board=uno_pic32 --board=esp01 --board=nodemcuv2 --board=esp32dev
platformio ci --project-option='build_flags=-D ${{ env.TINYGSM_MODEM_TO_USE }}' --project-option='framework=arduino' --board=esp32dev --board=esp32s3box
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed other boards which are not compatible and not used by Lilygo

"platforms": "*",
"frameworks": "arduino",
"platforms": [
"espressif32"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix this fork compatibility since other board compatibility is not ensured in new implementations

@@ -35,6 +35,7 @@ typedef TinyGsmSim7000::GsmClientSim7000 TinyGsmClient;
#include "TinyGsmClientSIM7020.h"
typedef TinyGsmSim7020 TinyGsm;
typedef TinyGsmSim7020::GsmClientSim7020 TinyGsmClient;
typedef TinyGsmSim7020::GsmClientSecureSim7020 TinyGsmClientSecure;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test was failing because 7020 is declared (macro) to support SSL, but the typedef was missing

@@ -508,7 +508,7 @@ class TinyGsmA7670 : public TinyGsmModem<TinyGsmA7670>,
waitResponse();
}
sendAT(GF("+CGNSSPWR=1"));
if (waitResponse(10000UL,"+CGNSSPWR: READY!") != 1) { return false; }
if (waitResponse(10000UL, GF("+CGNSSPWR: READY!")) != 1) { return false; }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation failure here: I guess you meant GF ? Function called was the one with a String buffer for the returned value.

@@ -488,7 +488,7 @@ class TinyGsmSim7600 : public TinyGsmModem<TinyGsmSim7600>,
}
sendAT(GF("+CGPS=0"));
if (waitResponse() != 1) { return false; }
return waitResponse(30000UL,"+CGPS: 0") == 1;
return waitResponse(30000UL,GF("+CGPS: 0")) == 1;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation failure here: I guess you meant GF ? Function called was the one with a String buffer for the returned value.

Comment on lines +184 to +186
uint8_t status = 0;
modem.getGPS(&status, &latitude, &longitude);
modem.getGPS(&status, &latitude, &longitude, &speed, &alt, &vsat, &usat, &acc, &year,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation failure also here: the test was not adapted following the addition of the status parameter

Copy link
Author

@mathieucarbou mathieucarbou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lewisxhe : could you please have a look at this PR ?
Thanks!

@lewisxhe
Copy link
Owner

@mathieucarbou Sorry, if you don't @ me, I may never look at this fork, haha, I don't follow this fork, I forked it from @vshymanskyy to add functions exclusive to the LilyGo board, just for the convenience of users who purchase LilyGo , since @vshymanskyy did not promote the development of TinyGSM in the later period, I did not submit a PR.
Regarding the PR you submitted, I need to carefully check whether it will affect LilyGo's current usage examples. Give me some time and I will take a look at it on Monday. Thank you for your enthusiasm, after all, few people do this now.

@mathieucarbou
Copy link
Author

mathieucarbou commented Mar 16, 2024

@mathieucarbou Sorry, if you don't @ me, I may never look at this fork, haha, I don't follow this fork, I forked it from @vshymanskyy to add functions exclusive to the LilyGo board, just for the convenience of users who purchase LilyGo , since @vshymanskyy did not promote the development of TinyGSM in the later period, I did not submit a PR. Regarding the PR you submitted, I need to carefully check whether it will affect LilyGo's current usage examples. Give me some time and I will take a look at it on Monday. Thank you for your enthusiasm, after all, few people do this now.

Thanks!

Yes I understood that also: the original repo seems not updated and it makes sense to keep Lilygo additions / updates here. That is why I intended to send Prs here because I am using 2 Lilygo boards (SIM7080G and T-A7670G) and I saw some issues worth to fix in this repo for Lilygo users. That's also why I focused the CI to only Lilygo boards.

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

Successfully merging this pull request may close these issues.

2 participants