-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
BDsensor: adding support for new probe sensor BDsensor #6490
base: master
Are you sure you want to change the base?
Conversation
Thanks a lot for your contribution. Please also provide the signed off as per point 3 in "What to expect in a review" in master/docs/CONTRIBUTING.md Also, it would need to come with an update of the documentation in the relevant chapters, e.g. "bed level" and "g-codes" |
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.
cant review/test in a technical perspective.. just some code-quality/hygiene hints :)
I guess this project could need some proper linting rules ;)
src/BD_sensor.c
Outdated
uint32_t delay_m = 20, homing_pose = 0; | ||
int sda_pin = -1, scl_pin = -1, z_ofset = 0; | ||
uint16_t BD_Data; | ||
//extern uint32_t timer_period_time; |
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 be removed? =)
src/BD_sensor.c
Outdated
|
||
///////////BDsensor as endstop | ||
struct timer time_bd; | ||
#include "autoconf.h" |
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.
maybe move it up to all other imports?
src/BD_sensor.c
Outdated
} | ||
void ndelay_bd(uint32_t nsecs) |
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.
} | |
void ndelay_bd(uint32_t nsecs) | |
} | |
void ndelay_bd(uint32_t nsecs) |
src/BD_sensor.c
Outdated
ndelay_bd_c(nsecs); | ||
} | ||
|
||
unsigned short BD_Add_OddEven(unsigned short byte) |
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.
a little bit confusing to name a "short" byte
=)) was wondering how a byte can have more than 8 (10) bits looking at the loop :)
src/BD_sensor.c
Outdated
} | ||
void BD_i2c_stop(void) |
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.
} | |
void BD_i2c_stop(void) | |
} | |
void BD_i2c_stop(void) |
src/BD_sensor.c
Outdated
uint16_t BD_i2c_read(void) | ||
{ | ||
uint16_t b = 1024; | ||
// if(BD_read_flag==1014) |
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 be removed? :)
src/BD_sensor.c
Outdated
else | ||
b=1024; | ||
|
||
#if 0 |
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 be removed? or maybe add a speaking comment why this is present :)
src/BD_sensor.c
Outdated
uint8_t data[8]; | ||
uint16_t BD_z; | ||
|
||
//if(BD_read_flag==1018) |
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 be removed?
{ | ||
int addr=atoi((char *)args[2]); | ||
BD_read_flag=addr; | ||
if(addr==1015) |
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.
maybe all these const's should be an enum? 1015, 1023, 1024, .. so many number all over the place :))
Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html There are some steps that you can take now:
Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available. Best regards, PS: I'm just an automated script, not a human being. |
Thanks. Is this sensor based on the ldc1612 chip? There has been some work done on that chip at #6536 (and #6537). My high-level feedback is that I think we need to integrate these efforts into a single "probe_eddy_current.py" type of module. I think we want to avoid having each probe implement their own implementations of PROBE, PROBE_ACCURACY, homing, etc. -Kevin |
hello Kevin, thanks! beside it can output the distance value or raw DAC data via the i2c port, it can also work as a switch sensor (just output low/high level in the sda pin) while homing, that helps to consumes low mcu usage. with the new feature nozzle collision sensing that use the raw DAC value to adjust the z_offset automatically.
|
I agree, and it is no problem to remove the PROBE and PROBE_ACCURACY in the BDsensor.py and use the PROBE, PROBE_ACCURACY in the probe.py. |
adding support BDsensor for Fast auto bed leveling leveling.
About BDsensor:
It can measure the bed distance at any point in real time without moving the z axis up and down.
now the fast auto bed leveling is stable for pull, there will be more new features like real time adjust and strain gauge in the future.https://github.com/markniu/Bed_Distance_sensor
Config: https://github.com/markniu/Bed_Distance_sensor/wiki/Installing-for-Klipper