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.

Sign up for GitHub

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

Jump to bottom

Added MQTT client, Maintenance tap feature, C++ style rework, Tasks #3

Merged
merged 50 commits into from Apr 29, 2023
Merged

Added MQTT client, Maintenance tap feature, C++ style rework, Tasks #3

merged 50 commits into from Apr 29, 2023

Conversation

PBrunot
Copy link
Contributor

@PBrunot PBrunot commented Apr 10, 2023

  • removed secrets.h

@PBrunot
implementing comments from #1
a9319c4 
see #1
except const expr for compatibility reasons with base library LCD

applied code formatting + esp32s3 testing
@PBrunot
refactoring LCDWrapper
b1f59ee 
the LCDWrapper will decide what to write in each row based on current state, and update function arguments.
main function has only to update the state
@PBrunot
platformio config for both ESP32 + ESP32S3
60c4bde 
pins.h config via #define
comments improvement
Update README.md
@PBrunot
refactoring and PR #2 comments implementation
b798b37 
replaced enum with enum class

added BoardState class which wraps lcd, rfid, current user, server, machine and provides higher level function to the main.cpp

comments implementation of PR #2

memcpy instead of for loop

removed underscores for private members

init inline in the header

consistent usage of this->
@PBrunot
struct instead of long LCD parameter list
9d77b1a 
+ init in constructor
+ removed trailing _ for private members
@PBrunot
testing on esp32s3
ab3bdc8
@PBrunot
split RFID into a separate class
5ca8de0
@PBrunot
moved config/enum inside main class
e165ba3
@PBrunot
eliminating pointers (WIP)
3d44ade
@PBrunot
removed pointers
99f885d 
LCD setRow is now public
LCD text logic moved from LCD to BoardState
@PBrunot
struct and constexpr for pins definition
9cb7eb5 
#2 (comment)
@PBrunot
created struct for MachineId instead of uint16_t
215589d 
#2 (comment)
@PBrunot
VSCode formatting
8c64623
@PBrunot
removed copy constructors
222ba5c
@PBrunot
removed pointers and allocation in constructor
9523fb7
@PBrunot
first working version on ESP32s3
1bd69f0 
bugfix user_name display
tested with ESP32S3
it reads RFID tag, matches against whitelist, updates the LCD as expected

debugging

LCDWrapper : eliminated casts and connection status member, added pretty print for debugging
@PBrunot
AuthProvider class + testing
9220f68 
removed RFIDWrapper::SetUid
it works fine so far with LCD+RFID reader.
testing required for machine output enable, wrong rfid tags
still missing server interactions
@PBrunot
added maintenance and allowed flags for Machine
72ce28e 
allows the server to return to the arduino client the status of the machine: if maintenance is needed, and if usage of the machine is allowed.
board now checks server at startup and tries to get up-to-date machine information.
server returns name of holder if uid is valid.
not sure if the actual usage time of the machine shall be monitored by arduino (maybe based on one RUNNING signal from the machine) or done purely server-side, based on login/logout times.
@PBrunot
added nocard delay
42eca17
@PBrunot
added delay for powering off and icon
90f61f8 
added new icons to show the power status to the machine (on, off, powering off delay).
added new constants to configure the delays in conf::machine
added provision for audible warning before power-off in main

bugfix

wifi testing

bugfix

Update LCDWrapper.tpp

added sample serial log
@PBrunot
removed secrets.h
6fcc289
@PBrunot
Merge branch 'main' of https://github.com/PBrunot/rfid-arduino
dc4242b
@PBrunot
added FabUser::UserLevel, added maintenance block
2ac456c 
implementing comments of issue #4
whitelist allows hardcoding of both UID and user priviledge
fixed power off delay bug

forced compilation to C++17 for std::string_view

auto logout

Update secrets.h.example
@PBrunot
usage of std::optional and std::find_if
12465a7 
instead of struct with bools and for loops over arrays.
applied consistently (millis() - start > DELAY) instead of other comparisons which are less unsigned int arithmetic safe.

formatting + constexpr

constexpr configuration strings with std::string_view
@PBrunot
code & names cleanup
a2c2ec3 
Used "FabUser" consistently
Renamed member_uid as card_uid
split the card namespace into separate file
@PBrunot
added comments
7c32787
@PBrunot
added beep and comments
b3cc92e
@PBrunot
maintenance WIP
9050259
@PBrunot
beep, relay, diagnostics debug
32f7b2c
@PBrunot
Merge branch 'main' of https://github.com/PBrunot/rfid-arduino
1ffcbe9
@PBrunot
Update LCDWrapper.tpp
6a0d727 
Update main.cpp
@PBrunot
Update main.cpp
49e1cc5
@PBrunot
bugfixes
2da8a83
@PBrunot
const more things, struct bindings
0cc7f2f
@PBrunot
improving c++ guidelines adherence
855df9c 
more const + LCD backlight method implementation
@PBrunot
Update LCDWrapper.h
b8981db
@PBrunot
applied camelCase on class members
b0f1107
@PBrunot
added logging
368cf05
@PBrunot
moved static global variables into boardstate
b3a9a75 
+used reference passing on structs
+ some renaming to avoid variables with same name as namespace Board
@PBrunot
improving C++ style
379214e 
avoid mixing unsigned and int arithmetic
using for range constructs
fixed bugs for LCDs with different configuration than 16x2
using string_stream to build larger strings
@PBrunot
Update AuthProvider.cpp
f0d33e1
@PBrunot
removed raw pointers for std::unique_ptr
0c53680
@PBrunot
Replaced LCDWrapper<>::Config with pins_conf::lcd_config
4e7edbf
@PBrunot
added long tap for maintenance
d671bd5 
added debug flag
bugfixes
@PBrunot
bugfix whitelist
b4d042c
@PBrunot PBrunot mentioned this pull request Apr 22, 2023
Open
valerionew
valerionew reviewed Apr 22, 2023
View reviewed changes
src/BoardState.cpp Outdated Show resolved Hide resolved
@PBrunot
CI with platformio
e083839 
CI tests

Update platformio.yml

Create super-linter.yml

Delete super-linter.yml

Create linter.yml

Update linter.yml

Update platformio.yml

Delete linter.yml

Update README.md

Update platformio.yml
@PBrunot
linter tests
138deb0
@PBrunot
fixing linter warnings
ec57145
@PBrunot
Squashed commit of the following:
349154f 
commit e82ad83
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Mon Apr 24 16:28:19 2023 +0200

    LongTap

    improved LongTap feature

    added interactive display
    example in log file

commit 470bc84
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Mon Apr 24 15:29:58 2023 +0200

    refactored BoardState into BoardLogic

    only one class Board::BoardLogic

commit c9dc3d2
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 22:17:49 2023 +0200

    linter warnings fixes

    linter warnings fixes

    linter warnings

    all files with 2 spaces ident

    using llvm clangtidy options

    Update main.cpp

    renamed Board.h as globals.h

    because that's just what it is

commit 9dd9138
Merge: 965df35 eeca1de
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 21:29:47 2023 +0200

    Merge branch 'asynv' of https://github.com/PBrunot/rfid-arduino into async

commit 965df35
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 21:29:13 2023 +0200

    Update RFIDWrapper.cpp

commit 60f87dd
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:35:09 2023 +0200

    moved Wifi init to BoardState::init

commit 6bf6c1b
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:15:55 2023 +0200

    Update main.cpp

commit b806094
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:04:49 2023 +0200

    refactoring

    moving code from BoardState to BoardLogic
    renamed DEBUG to ENABLE_LOGS
    moved BoardState constants to conf::buzzer
    tested with real hardware

commit 43fbbdc
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 11:42:15 2023 +0200

    rewritten with arkhipenko/TaskScheduler

commit 81c2634
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 09:59:35 2023 +0200

    trying async pattern

commit eeca1de
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:35:09 2023 +0200

    moved Wifi init to BoardState::init

commit a366e8f
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:15:55 2023 +0200

    Update main.cpp

commit 0dcc714
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 19:04:49 2023 +0200

    refactoring

    moving code from BoardState to BoardLogic
    renamed DEBUG to ENABLE_LOGS
    moved BoardState constants to conf::buzzer
    tested with real hardware

commit 5ad8359
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 11:42:15 2023 +0200

    rewritten with arkhipenko/TaskScheduler

commit ee4e211
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 09:59:35 2023 +0200

    trying async pattern

commit 565689a
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 17:15:53 2023 +0200

    Update platformio.yml

commit 9bda291
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 17:06:06 2023 +0200

    Update README.md

commit 01f5aeb
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 17:02:40 2023 +0200

    Update platformio.yml

commit cf9f9f3
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Sun Apr 23 16:58:56 2023 +0200

    CI with platformio
@PBrunot
Copy link
Contributor Author

PBrunot commented Apr 24, 2023

Notable changes in this PR:

  • I have used a cooperative task scheduler to simplify loop() with several independent tasks.
  • I configured platform.io cland-tidy as linter and removed many warnings, so C++ code shall look decent now
  • Configured clang-tidy integration in platform.io ; but it has a bug on windows ( pio check using clangtidy fails on large project platformio/platformio-core#4446 ). Build still works on Windows with VSCODE+Platform.io extension.
  • I created a GitHub action to automatically use platformio to build on push and archive BIN files. They get deleted after 10 days.

Still missing for V1.0:

  • server integration (mock functions only)
  • some automated tests.

@valerionew
Copy link
Member

This is excellent progress, congrats! 🤩

For the server-machine communication, should we stick with the MQTT protocol that was foreseen by me and @lorossi?

An issue I can see with the automated builds is that:

  • secrets should not be distributed publicly with the compiled bin (very easy to reverse engineer);
  • a compiled bin without the secrets is not really useful.

So I don't really know if there is a good way to solve this. Is private artifacts a thing?

@PBrunot
Copy link
Contributor Author

PBrunot commented Apr 24, 2023

@valerionew : ok, I can implement a MQTT client and move on this topic.

Right now the builds are based on secrets.h.example which do not contain any useful information.
I don't see a good way to protect the contents for automated builds. I mean, if the goal is to have one standard image, and make this public, we can't have a custom list of RFID badge in the image, even if encrypted somehow.

On the other hand, having a whitelist is really a good idea to make the solution reliable even in case of network/server failures.

One option could be to add a method to the server, to provide a list of whitelisted users (maybe all the fablab staff priviledged users), and then we could persist this on ESP32 flash memory. This requires the ESP32 to be online with the server at least once... What do you think?

@PBrunot
Squashed commit of the following:
6fd2aa1 
commit 66ba233
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Thu Apr 27 20:19:38 2023 +0200

    C++ style, removed const std::string_view

    as they are passed by value

    C++17 if with initializers

    code cleanup

commit c5517de
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Tue Apr 25 23:13:42 2023 +0200

    fixed 2 linter warnings

commit b930693
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Tue Apr 25 23:07:39 2023 +0200

    Update secrets.h.example

commit c8f178e
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Tue Apr 25 18:52:20 2023 +0200

    tested MQTT checkmachine/checkuser/startuse/stopuse

    MQTT testing

commit 55b4e15
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Tue Apr 25 14:50:30 2023 +0200

    MQTT WIP

commit f0985b2
Author: Pascal Brunot <pascal.brunot@gmail.com>
Date:   Mon Apr 24 22:22:58 2023 +0200

    MQTT client WIP

    FabServer / mqtt classes WIP
@PBrunot PBrunot changed the title Added regular WiFi polling. Added MQTT client, Maintenance tap feature, C++ style rework, Tasks Apr 29, 2023
@PBrunot PBrunot merged commit d33eb65 into fablab-bergamo:main Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valerionew valerionew valerionew left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@PBrunot @valerionew