-
Notifications
You must be signed in to change notification settings - Fork 426
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
fixed user pincode, added mqqt discovery for more data, other minor changes #644
Conversation
Thanks. Much appreciated. |
Hey @pvtex thank you :) Can you please try rebasing these commits on top of If you could also possibly reduce the number of commits it would help us better review the code. Otherwise just resolving the conflicts is fine :) Thanks! |
@pvtex if you struggle with Git we can have a call and figure out things together, I'm very interested in having a look at your code. Feel free to send me an email if you want to go this way, you find the address in my Github profile. Cheers |
I am right now rebasing my reposotory to the device branch. But could take some time |
d10e49b
to
55d6801
Compare
commit 55d6801 Author: wasn-eu <[email protected]> Date: Wed Jun 5 16:42:09 2024 +0200 clean up
squashed all my commits and rebased my repo on dev branch |
Cool, thanks, can you please delete the .pio and ESP-Debug folders from your PR? |
deletet the folders .pio and ESP-Debug, |
Sorry, I'm not getting what are you trying to achieve with this PR. Can you please provide a description of the change? Also, it would be better to have one PR for one feature, to make it easier to review and discuss, so if there are multiple things here can we start from just one? |
mainly these things are changed/added:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just had time to give another (I think last!) round of comment, if you have time to have a look at this. Thanks!!
Exception.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please delete this file?
src/websrc/index.html
Outdated
@@ -34,6 +34,7 @@ | |||
<div class="sidebar-header"> | |||
<br> | |||
<h1><i class="glyphicon glyphicon-tags" aria-hidden="true"></i>esp-rfid</h1> | |||
<center<<p>RFID-Haustuer</p></div>center> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please remove this?
src/websrc/js/esprfid.js
Outdated
@@ -2033,7 +2033,7 @@ function login() { | |||
|
|||
function getLatestReleaseInfo() { | |||
|
|||
$.getJSON("https://api.github.com/repos/esprfid/esp-rfid/releases/latest").done(function(release) { | |||
$.getJSON("https://api.github.com/repos/esp-rfid/esp-rfid/releases/latest").done(function(release) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not be changed
offset_hours = offset_hours * 3600; // convert number of days to seconds | ||
t_unix_date = accesstime + offset_hours; | ||
char accesstimestr[9]; | ||
sprintf(accesstimestr, "%02d.%02d.%4d %02d:%02d:%02d\n", day(t_unix_date), month(t_unix_date), year(t_unix_date), hour(t_unix_date), minute(t_unix_date), second(t_unix_date)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pvtex can you please remove this time conversion to string? I agree that is more human readable, but we would change the MQTT API by doing this and it becomes more difficult to parse by machines
offset_hours = offset_hours * 3600; // convert number of days to seconds | ||
t_unix_date = accesstime + offset_hours; | ||
char accesstimestr[9]; | ||
sprintf(accesstimestr, "%02d.%02d.%4d %02d:%02d:%02d\n", day(t_unix_date), month(t_unix_date), year(t_unix_date), hour(t_unix_date), minute(t_unix_date), second(t_unix_date)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please remove this as well? Same applies as above
@@ -34,6 +34,7 @@ | |||
<div class="sidebar-header"> | |||
<br> | |||
<h1><i class="glyphicon glyphicon-tags" aria-hidden="true"></i>esp-rfid</h1> | |||
<div id="systemname" style='text-align: center;'></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please use class="text-center"
instead of the inline style?
I'm closing this since it's abandoned and I continue here: #654 |
No description provided.